I've refactored my tree package several times, and not found a solution I'm happy with, so I'd like a little advice on the best way to proceed.
I've tried to reduce the problem down to it's essence, and made a simple example of a tree which is composed of nodes. All nodes have a set of common functionality (expressed in the example as an open / closed state). In addition, there are several types of nodes, each with specialist behaviours (expressed in the example as editable nodes implementing EditorInterface and having a visible / hidden state).
In my example we try to satisfy the desired behaviour - any node can be opened, and when it's opened if it's editable, it should make the editor visible.
My example defines two types of node, folders and documents. Documents are editable.
My instinct is to define a struct for node, and include common functionality as members and methods. Then define structs for folder and document, with an embedded anonymous node struct in each.
However, this causes a problem which will be highlighted by my first example. I've created a simple test that fails:
Example 1: https://play.golang.org/p/V6UT19zVVU
// In this example the test fails because we're unable to access the interface in SetNodeState.
package main
import "testing"
func TestTree(t *testing.T) {
n := getTestNode()
n.SetNodeState(true)
if !n.(*document).visible {
t.Error("document is not visible")
}
}
func getTestNode() NodeInterface {
doc := &document{node: &node{}, content: "foo"}
folder := &folder{node: &node{children: []NodeInterface{doc}}, color: 123}
return folder.children[0]
}
type NodeInterface interface {
SetNodeState(state bool)
}
type EditorInterface interface {
SetEditState(state bool)
}
type node struct {
open bool
parent NodeInterface
children []NodeInterface
}
func (n *node) SetNodeState(state bool) {
n.open = state
// TODO: obviously this isn't possible.
//if e, ok := n.(EditorInterface); ok {
// e.SetEditState(state)
//}
}
type folder struct {
*node
color int
}
var _ NodeInterface = (*folder)(nil)
type document struct {
*node
visible bool
content string
}
var _ NodeInterface = (*document)(nil)
var _ EditorInterface = (*document)(nil)
func (d *document) SetEditState(state bool) {
d.visible = state
}
I've tried to refactor this several times to achieve the desired behaviour, but none of the methods makes me happy. I won't paste them all into the question, but I've created Go playground links:
Example 2: https://play.golang.org/p/kyG-sRu6z- In this example the test passes, because we add the interface as the "self" member of the embedded struct. This seems like a nasty kludge.
Example 3: https://play.golang.org/p/Sr5qhLn102 In this example, we move SetNodeState to a function that accepts the interface. The disadvantage of this is that we don't have access to the embedded struct, so all members need getters and setters exposed on the interface. This makes the interface needlessly complex.
Example 4: https://play.golang.org/p/P5E1kf4dqj In this example, we provide a getter to return the entire embedded struct, which we use in the SetNodeState function. Again this seems like a nasty kludge.
Example 5: https://play.golang.org/p/HMH-Y_RstV In this example we pass in the interface as a parameter to every method that needs it. Again, this doesn't feel right.
Example 6: https://play.golang.org/p/de0iwQ9gGY In this example, we remove NodeInterface, and construct nodes from a base struct and an object implementing ItemInterface. This is perhaps the least problematic of the examples, but still leaves me wanting a better solution.
Perhaps someone can suggest a better solution?
Here, I'd have document nodes reimplement SetNodeState, and use d.node.SetNodeState to update the node's state; in non-Go-y terms, I'd push the class-specific code down to the subclass, like this:
package main
import "testing"
func main() {
tests := []testing.InternalTest{{"TestTree", TestTree}}
matchAll := func(t string, pat string) (bool, error) { return true, nil }
testing.Main(matchAll, tests, nil, nil)
}
func TestTree(t *testing.T) {
n := getTestNode()
n.SetNodeState(true)
if !n.(*document).visible {
t.Error("document is not visible")
}
}
func getTestNode() NodeInterface {
doc := &document{node: &node{}, content: "foo"}
folder := &folder{node: &node{children: []NodeInterface{doc}}, color: 123}
return folder.children[0]
}
type NodeInterface interface {
SetNodeState(state bool)
}
type node struct {
open bool
parent NodeInterface
children []NodeInterface
}
func (n *node) SetNodeState(state bool) {
n.open = state
}
type folder struct {
*node
color int
}
var _ NodeInterface = (*folder)(nil)
type document struct {
*node
visible bool
content string
}
func (d *document) SetNodeState(state bool) {
d.node.SetNodeState(state)
d.SetEditState(state)
}
func (d *document) SetEditState(state bool) {
d.visible = state
}
This also lets you write the general methods that apply to any node without referring to specific node types, which you might find cleaner than an approach where node methods have type assertions for particular types.
(That, in turn, would let you make a public Node/NodeInterface and keep them in a separate package from specific node types, since the specific types would only depend on the general type and never the other way around (recall two Go packages can't both depend on each other). But it seems reasonable to keep the node type together in a package with the specific node types as you're doing.)
Where the above approach doesn't apply, something like your third example (having a function that takes the interface) might help. To shorten it a bit, that interface might be able to provide just getNode() *node rather than setOpen, appendChild, etc., depending on specifics of the situation.
The Go stdlib exports functions that take interfaces, with, e.g., io.ReadFull(r, buf) instead of Readers having a ReadFull(buf) method. I suspect it'd be considered bad form in, say, C++ for the code to be in a bare function rather than a method, but it's a common practice in Go.
So: sometimes you can get OO-ish behavior by (re)implementing a method on the specific type; when you can't, functions accepting an interface are idiomatic.