Add support for Trie.Walk() and Node.SetValue()

[shivamMg]

Extending on work done here: #3

[tophe]

hi,

I have push on my fork some comments, and update the check so, it can check the node.SetValue() during the Walk.
We can do a search with the Walk function, but that's not trivial, and the func signature doesn't help to understand you can do search like that.

The Filter func is easier to understand, because it return a searchResult and that what you expect with a search by value. You have a simple signature with a selector function. I have rewrite the filter, as you do the walk.

look at filter.go

[shivamMg]

I think I disagree. Walk() functions are pretty ubiquitous in tree-like structures e.g. filepath.Walk, trie.Walk, tour example.

Yes, filter function that returns the result list is convenient - users don't have to write code to append to a result that they would've to do with Walk func. But Walk func in general gives users more control over what to do in the traversal - one of them being filtering. Therefore, it's a super-feature to filtering.

Example of when Walk func will be faster than filtering+trie.Put would be when you need to update value in tree traversal:

tri.Walk(nil, func (key []string, node *Node) error {
  if node.Value() == 2 {
    node.SetValue(3)
  }
  return nil
})

[tophe]

Yes I agree Walk function is great to update tree along the traversal, because it is it main purpose.
Selecting with the Walk function is ugly because it break function encapsulation. You update a variable that's it is in outer scope of the function, you have to read all the code to understand how that magic happen because the function return an error, and you want a result.

        var selected []string
	walker := func(key []string, node *trie.Node) error {
		what := node.Value().(int)
		if what == 3 {
			selected = key
			return errors.New("found")
		}
		return nil
	}

the function use selected variable define in is outer scope, as if this was a closure, it's hard to understand. Whereas the filter function return a result, and it is quite ubiquitous to many language (ruby , js ).

the 2 functions have differents usage, sure you can do select and update with Walk, but you to explain usage in documentation, because that's hard to figure.

[shivamMg]

I don't think Walk/Anonymous functions are ugly. Only when the anon function definition becomes huge it becomes a code smell. At which point it can be moved to a separate function.

Overall, I feel that Filter functionality should be written by the users as required using Walk. Don't feel it needs to be part of the library. So, I'm going to merge this PR now.

func Filter(tri *trie.Trie, f func(interface{}) bool) (filtered []*trie.SearchResult) {
	_ = tri.Walk(nil, func(key []string, node *trie.Node) error {
		if f(node.Value()) {
			filtered = append(filtered, &trie.SearchResult{
				Key:   key,
				Value: node.Value(),
			})
		}
		return nil
	})
	return
}

[tophe]

hi,
I take a look to other go walk function, it seem a common pattern to use it as you do, surely a good choice.
thank you for your help.