Refactoring Outputs to implement a standard interface #15
Conversation
|
@jtopjian, cool! I have questions! I'll ponder for a few. |
| Config Config | ||
| } | ||
|
|
||
| func (o *InvalidOutput) GenerateOutput(elements interface{}) (string, error) { |
There was a problem hiding this comment.
So, here you're 'struct typing', rather than explicitly declaring an type Outputer interface, right?
| "sort" | ||
| "strings" | ||
| ) | ||
| type OutputBuilder interface { |
There was a problem hiding this comment.
ah, so we are declaring a public interface here... This is intended to communicate to implementors of new output types that their output struct, say xmlOutput, should minimally receive a GenerateOutput method, right?
There was a problem hiding this comment.
I believe implementations have to match exactly the same number of methods as well as exact input and output types.
In this case, if a struct would implement anything other than GenerateOutput, it would no longer conform to the interface.
There was a problem hiding this comment.
I think 'struct typing' allows any struct that minimally implements the interface, but may implement methods outside the interface, to be used as the interface. For example: https://play.golang.org/p/2JS1qC9kca
There was a problem hiding this comment.
Ah - I did not know that :)
| } | ||
| default: | ||
| parsed[label] = fmt.Sprintf("%v", elements) | ||
| return label, nil, parsed | ||
| v = &InvalidOutput{ |
There was a problem hiding this comment.
Safe zero value/Null Object pattern. Makes sense.
|
Overall, this is intended to communicate more strongly with future implementors of novel output methods? Are there compile-time type-checking benefits with this approach that we don't currently have? |
This is one case, yes. I think compared to the original implementation, it's sort of six-one-way-half-a-dozen-the-other, especially considering all of the non-method functions scattered around. However, I'm slightly in favor of the interface style simply because having several functions like I think if the switch statements were removed in both cases, it would make a lot more sense -- true struct/duck typing at that point. I haven't figured out how to cleanly do that yet, though, or if it's even possible. These two particular cases are interesting, though, because they're the first I've run into where using an interface makes slightly more sense than "I think it'd be cool to do this". slightly :) Anyway, I'm on the fence. I think I might just keep this branch around to reference in the future if something else pushes toward this. |
Right. Somehow |
@erichs This is slightly overkill for outputs, but I think there's some value for the cloud configuration. Thoughts?