add tree traversal in golang #406
Conversation
june128
added
the
Implementation
|
Hey @Liikt, I trust you, but we are lacking Go expertise. Do you mind if I do some of these reviews and ask some dumb questions about Go, just to make sure I understand the syntax? |
| children []*node | ||
| } | ||
|
|
||
| func dfsRecursive(n *node) { |
There was a problem hiding this comment.
Consider returning an array of elements or taking a visitor function, rather than just printing all of them.
That goes for all of these functions, not just this one.
There was a problem hiding this comment.
I rather have a pointer since it is more memory efficient. I also find it way easier. Tbh i'm not really sure why an array would be better, since now i can build a native tree structure and don't have to build an array and do crazy offset math to figure out the children.
There was a problem hiding this comment.
@Liikt Yeah, that got shot down in other places too, since we're just demonstrating the algorithm, not trying to build reusable chunks of code. Disregard this one.
| fmt.Println(n.id) | ||
| default: | ||
| fmt.Println("This is not a binary tree") | ||
| } |
There was a problem hiding this comment.
This can be simplified a bit by running code if lengths are greater than a certain amount, rather than just if they're equal to it. I'm not confident enough with Golang to suggest an alternative without an interpreter handy, but in pseudocode:
if children.length > 2:
throw "Not a binary tree"
if children.length > 1:
inOrder(children[1])
printThis()
if children.length > 0:
inOrder(children[0])
There was a problem hiding this comment.
I agree with you here but I prefer the switch just for readability. It can definitely be made shorter like that though.
There was a problem hiding this comment.
@Liikt Yup! Which is why I still approved. Both of these were ideas, not must-fixes.
There was a problem hiding this comment.
If @nic-hartley approves, so do I. Thanks a bunch for the review!
compiled with
go version go1.11 linux/amd64and without errors