Skip to main content
Code Review

Return to Answer

added 2362 characters in body
Source Link
Martin R
  • 24.1k
  • 2
  • 37
  • 95

The code then looks like this:

class WordDictionary {
 var root: TrieNode
 var cache: [String: Bool] = [:]
 var cacheValid = true
 
 class TrieNode {
 var next = [Character: TrieNode]()
 var isWordEnd = false
 }
 
 init() {
 root = TrieNode()
 }
 
 func addWord(_ word: String) {
 var node = root
 
 for char in word {
 node = addInternal(char, at: node)
 }
 
 node.isWordEnd = true
 cacheValid = false
 }
 
 func addInternal(_ char: Character, at node: TrieNode) -> TrieNode {
 if let nextNode = node.next[char] {
 return nextNode
 }
 
 let nextNode = TrieNode()
 node.next[char] = nextNode
 return nextNode
 }
 
 func search(_ word: String) -> Bool {
 if (!cacheValid) {
 let falseKeys = cache.compactMap { 0ドル.value == false ? 0ドル.key : nil }
 for key in falseKeys {
 cache[key] = nil
 }
 cacheValid = true
 } else if let result = cache[word] {
 return result
 }
 
 func dfs(index: String.Index, at node: TrieNode) -> Bool {
 var currentNode = node
 var currentIndex = index
 
 while currentIndex < word.endIndex {
 let currentChar = word[currentIndex]
 
 if currentChar == "." {
 for nextNode in currentNode.next.values {
 let nextIndex = word.index(currentIndex, offsetBy: 1)
 if dfs(index: nextIndex, at: nextNode) {
 return true
 }
 }
 return false
 }
 
 if let nextNode = currentNode.next[currentChar] {
 currentNode = nextNode
 word.formIndex(after: &currentIndex)
 } else {
 return false
 }
 }
 
 return currentNode.isWordEnd
 }
 
 let result = dfs(index: word.startIndex, at: root)
 cache[word] = result
 return result
 }
}

The code then looks like this:

class WordDictionary {
 var root: TrieNode
 var cache: [String: Bool] = [:]
 var cacheValid = true
 
 class TrieNode {
 var next = [Character: TrieNode]()
 var isWordEnd = false
 }
 
 init() {
 root = TrieNode()
 }
 
 func addWord(_ word: String) {
 var node = root
 
 for char in word {
 node = addInternal(char, at: node)
 }
 
 node.isWordEnd = true
 cacheValid = false
 }
 
 func addInternal(_ char: Character, at node: TrieNode) -> TrieNode {
 if let nextNode = node.next[char] {
 return nextNode
 }
 
 let nextNode = TrieNode()
 node.next[char] = nextNode
 return nextNode
 }
 
 func search(_ word: String) -> Bool {
 if (!cacheValid) {
 let falseKeys = cache.compactMap { 0ドル.value == false ? 0ドル.key : nil }
 for key in falseKeys {
 cache[key] = nil
 }
 cacheValid = true
 } else if let result = cache[word] {
 return result
 }
 
 func dfs(index: String.Index, at node: TrieNode) -> Bool {
 var currentNode = node
 var currentIndex = index
 
 while currentIndex < word.endIndex {
 let currentChar = word[currentIndex]
 
 if currentChar == "." {
 for nextNode in currentNode.next.values {
 let nextIndex = word.index(currentIndex, offsetBy: 1)
 if dfs(index: nextIndex, at: nextNode) {
 return true
 }
 }
 return false
 }
 
 if let nextNode = currentNode.next[currentChar] {
 currentNode = nextNode
 word.formIndex(after: &currentIndex)
 } else {
 return false
 }
 }
 
 return currentNode.isWordEnd
 }
 
 let result = dfs(index: word.startIndex, at: root)
 cache[word] = result
 return result
 }
}
added 312 characters in body
Source Link
Martin R
  • 24.1k
  • 2
  • 37
  • 95

This can be optimized further:

  • Repeated calls to cache.removeAll() can be avoided. Instead of emptying the cash in addWord(), set an "invalid" flag and empty the cache in the search() method if that flag is set.

  • true entries in the cache remain valid if a new word is added to the dictionary. So instead of completely emptying the cache if a new word has been added, remove only the false entries.

This can be optimized further:

  • Repeated calls to cache.removeAll() can be avoided. Instead of emptying the cash in addWord(), set an "invalid" flag and empty the cache in the search() method if that flag is set.

  • true entries in the cache remain valid if a new word is added to the dictionary. So instead of completely emptying the cache if a new word has been added, remove only the false entries.

Source Link
Martin R
  • 24.1k
  • 2
  • 37
  • 95

Good job in incorporating the feedback from your previous question to improve the Trie implementation.

The dfs() function can be improved a bit: Creating an array of all successor nodes (in the case of a wildcard character)

if let nextNodes = Array(currentNode.next.values) as? [TrieNode] {
 for nextNode in nextNodes {
 // ...
 }
}

is not necessary, one can iterate over the values directly:

for nextNode in currentNode.next.values {
 // ...
}

The variable

var nextIndex = currentIndex
word.formIndex(after: &nextIndex)

can be made a constant with

let nextIndex = word.index(currentIndex, offsetBy: 1)

Also

if let nextNode = currentNode.next[currentChar] {
 currentNode = nextNode
 word.formIndex(after: &currentIndex)
 continue
}
return false

is in my opinion better written with an else block instead of continue:

if let nextNode = currentNode.next[currentChar] {
 currentNode = nextNode
 word.formIndex(after: &currentIndex)
} else {
 return false
}

Caching the results

In the test case that fails with a timeout error, the same string "ghkdnjmmgcddganjkigmabbho" is searched 5000 times. So caching does help. It can be easily added to the search function. Note that the cache must be invalidated if a new word is added to the dictionary:

class WordDictionary {
 var root: TrieNode
 var cache: [String: Bool] = [:]
 // ... 
 func addWord(_ word: String) {
 // ...
 cache.removeAll()
 }
 
 // ... 
 func search(_ word: String) -> Bool {
 if let result = cache[word] {
 return result
 }
 
 func dfs(index: String.Index, at node: TrieNode) -> Bool {
 // ...
 }
 
 let result = dfs(index: word.startIndex, at: root)
 cache[word] = result
 return result
 }
}
default

AltStyle によって変換されたページ (->オリジナル) /