perf(trie): remove recursive substring traversal

lib/natural/trie/trie.js

@@ -46,23 +46,21 @@ class Trie {
       string = string.toLowerCase()
     }
 
-    // If the string has only one letter, mark this as a word.
-    if (string.length === 0) {
-      const wasWord = this.$
-      this.$ = true
-      return wasWord
-    }
-
-    // Make sure theres a Trie node in our dictionary
-    let next = this.dictionary[string[0]]
+    const caseSensitive = this.cs
+    let node = this
 
-    if (!next) {
-      this.dictionary[string[0]] = new Trie(this.cs)
-      next = this.dictionary[string[0]]
+    for (let index = 0, length = string.length; index < length; index++) {
+      const character = string[index]
+      let next = node.dictionary[character]
+      if (!next) {
+        node.dictionary[character] = next = new Trie(caseSensitive)
+      }
+      node = next
     }
 
-    // Continue adding the string
-    return next.addString(string.substring(1))
+    const wasWord = node.$
+    node.$ = true
+    return wasWord
   }
 
   /**
@@ -94,9 +92,11 @@ class Trie {
     }
 
     function get (node, word) {
-      if (!node) return null
-      if (word.length === 0) return node
-      return get(node.dictionary[word[0]], word.substring(1))
+      for (let index = 0, length = word.length; index < length; index++) {
+        if (!node) return null
+        node = node.dictionary[word[index]]
+      }
+      return node || null
     }
 
     function recurse (node, stringAgg, resultsAgg) {
@@ -130,21 +130,15 @@ class Trie {
       string = string.toLowerCase()
     }
 
-    if (string.length === 0) {
-      return this.$
-    }
-
-    // Otherwise, we need to continue searching
-    const firstLetter = string[0]
-    const next = this.dictionary[firstLetter]
-
-    // If we don't have a node, this isn't a word
-    if (!next) {
-      return false
+    let node = this
+    for (let index = 0, length = string.length; index < length; index++) {
+      node = node.dictionary[string[index]]
+      if (!node) {
+        return false
+      }
     }
 
-    // Otherwise continue the search at the next node
-    return next.contains(string.substring(1))
+    return node.$
   }
 
   /**
@@ -161,26 +155,27 @@ class Trie {
       search = search.toLowerCase()
     }
 
-    function recurse (node, search, stringAgg, resultsAgg) {
+    const results = []
+    let node = this
+    const length = search.length
+
+    for (let index = 0; ; index++) {
       // Check if this is a word.
       if (node.$) {
-        resultsAgg.push(stringAgg)
+        results.push(search.slice(0, index))
       }
 
       // Check if the have completed the seearch
-      if (search.length === 0) {
-        return resultsAgg
+      if (index === length) {
+        return results
       }
 
       // Otherwise, continue searching
-      const next = node.dictionary[search[0]]
-      if (!next) {
-        return resultsAgg
+      node = node.dictionary[search[index]]
+      if (!node) {
+        return results
       }
-      return recurse(next, search.substring(1), stringAgg + search[0], resultsAgg)
-    };
-
-    return recurse(this, search, '', [])
+    }
   }
 
   /**
@@ -192,26 +187,33 @@ class Trie {
       search = search.toLowerCase()
     }
 
-    function recurse (node, search, stringAgg, lastWord) {
+    let node = this
+    let lastWordIndex = null
+    const length = search.length
+
+    for (let index = 0; ; index++) {
       // Check if this is a word
       if (node.$) {
-        lastWord = stringAgg
+        lastWordIndex = index
       }
 
       // Check if we have no more to search
-      if (search.length === 0) {
-        return [lastWord, search]
+      if (index === length) {
+        return [
+          lastWordIndex === null ? null : search.slice(0, lastWordIndex),
+          ''
+        ]
       }
 
       // Continue searching
-      const next = node.dictionary[search[0]]
-      if (!next) {
-        return [lastWord, search]
+      node = node.dictionary[search[index]]
+      if (!node) {
+        return [
+          lastWordIndex === null ? null : search.slice(0, lastWordIndex),
+          search.slice(index)
+        ]
       }
-      return recurse(next, search.substring(1), stringAgg + search[0], lastWord)
-    };
-
-    return recurse(this, search, '', null)
+    }
   }
 
   /**

spec/trie_spec.ts

@@ -194,4 +194,65 @@ describe('trie', function () {
       expect(results[1]).toBe('re')
     })
   })
+
+  describe('iterative traversal regressions', function () {
+    it('should add and find a very long string without overflowing the call stack', function () {
+      const trie = new Trie()
+      const value = 'a'.repeat(50000)
+      trie.addString(value)
+      expect(trie.contains(value)).toBe(true)
+    })
+
+    it('should preserve shared prefix results and order', function () {
+      const trie = new Trie()
+      trie.addStrings(['app', 'apple', 'application', 'apply'])
+      expect(trie.keysWithPrefix('app')).toEqual(['app', 'apple', 'application', 'apply'])
+    })
+
+    it('should preserve repeated insertion return values', function () {
+      const trie = new Trie()
+      expect(trie.addString('repeat')).toBe(false)
+      expect(trie.addString('repeat')).toBe(true)
+      expect(trie.contains('repeat')).toBe(true)
+      expect(trie.getSize()).toBe(7)
+    })
+
+    it('should preserve hit and miss search results', function () {
+      const trie = new Trie()
+      trie.addStrings(['their', 'there'])
+      expect(trie.contains('their')).toBe(true)
+      expect(trie.contains('theirs')).toBe(false)
+      expect(trie.contains('the')).toBe(false)
+    })
+
+    it('should preserve case-insensitive normalization in search results', function () {
+      const trie = new Trie(false)
+      trie.addStrings(['App', 'Apple', 'application', 'apply'])
+      expect(trie.contains('APPLICATION')).toBe(true)
+      expect(trie.findPrefix('ApPLICATIONS')).toEqual(['application', 's'])
+      expect(trie.findMatchesOnPath('ApPLICATIONS')).toEqual(['app', 'application'])
+    })
+
+    it('should preserve empty string behavior', function () {
+      const trie = new Trie()
+      expect(trie.addString('')).toBe(false)
+      expect(trie.addString('')).toBe(true)
+      expect(trie.contains('')).toBe(true)
+      expect(trie.findPrefix('missing')).toEqual(['', 'missing'])
+      expect(trie.findMatchesOnPath('missing')).toEqual([''])
+    })
+
+    it('should preserve exact findPrefix results', function () {
+      const trie = new Trie()
+      trie.addStrings(['their', 'and', 'they'])
+      expect(trie.findPrefix('theyre')).toEqual(['they', 're'])
+      expect(trie.findPrefix('theme')).toEqual([null, 'me'])
+    })
+
+    it('should preserve exact findMatchesOnPath order', function () {
+      const trie = new Trie()
+      trie.addStrings(['a', 'ab', 'bc', 'cd', 'abc'])
+      expect(trie.findMatchesOnPath('abcd')).toEqual(['a', 'ab', 'abc'])
+    })
+  })
 })