derekparker / trie

Data structure and relevant algorithms for extremely fast prefix/fuzzy string searching.
MIT License
748 stars 114 forks source link

Remove method fails for single / root key #4

Open codelinter opened 9 years ago

codelinter commented 9 years ago

See this http://play.golang.org/p/of8h4uJ5jR

After calling Remove, the trie still finds the key.

derekparker commented 9 years ago

It seems if you only have one key, the remove fails.

Good catch, I'll push a fix up soon.

codelinter commented 9 years ago

I slightly modified the code with else , which seems to solve this micro bug

func (t *Trie) Remove(key string) {

    var (
        i int

        rs = []rune(key)

        node = t.nodeAtPath(key)
    )

    t.size--

    for n := node.Parent(); n != nil; n = n.Parent() {
        // fmt.Printf("%T \n", n)
        i++

        if len(n.Children()) > 1 {

            r := rs[len(rs)-i]

            n.RemoveChild(r)

            break

        } else {
            if t.root != nil {
                t.root = &Node{}
            }
        }

    }

}
derekparker commented 9 years ago

I think I'd change the above to something like:

if n == t.root {
        t.root = &Node{}
}

if len(....) {
...

If you wouldn't mind putting together a PR and adding a test to catch the bug I'd definitely pull it in.

rench1988 commented 8 years ago
func (t *Trie) Remove(key string) {
    var (
        i    int
        rs   = []rune(key)
        node = findNode(t.Root(), []rune(key))
    )
    t.size--
    for n := node.Parent(); n != nil; n = n.Parent() {
        i++
        if len(n.Children()) > 1 {
            r := rs[len(rs)-i]
            n.RemoveChild(r)
            break
        }
    }
}

this will core dump when node is nil(key that isn't exist will crash)

jamierobertsusa commented 8 years ago

Derek -- I will put a PR together for this 1 - Remove panics if key doesn't exist 2 - Remove doesn't work on root key Do you want me to change the method signature to return bool ?