Added Questions asked in SWE - Intuit #136
Conversation
| Warning Rate limit exceeded@shubhamranswal has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 12 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThree new C++ solution files added: a sliding window algorithm finding the smallest substring containing all target characters, a Trie-based autocomplete system with insert/delete/update/search functionality, and a Trie-backed word break validator using dynamic programming. Changes
Sequence Diagram(s)sequenceDiagram participant User participant Autocomplete as Autocomplete System participant Trie participant Heap User->>Autocomplete: autocomplete(prefix, K) Autocomplete->>Trie: traverse to prefix node alt Prefix found Trie-->>Autocomplete: return node with wordFrequency map Autocomplete->>Heap: build max-heap from frequencies Heap-->>Autocomplete: top K entries sorted Autocomplete-->>User: return K most frequent words else Prefix not found Trie-->>Autocomplete: return null/empty Autocomplete-->>User: return empty vector end sequenceDiagram participant User participant WordBreak as Word Break Validator participant Trie participant DP as DP Array User->>WordBreak: wordBreak(s, dictionary) WordBreak->>Trie: insert all dictionary words Trie-->>WordBreak: Trie ready WordBreak->>DP: initialize dp[0] = true loop for each position i in string loop for each j < i alt dp[j] is true AND s[j..i) in Trie WordBreak->>DP: set dp[i] = true end end end DP-->>WordBreak: return dp[n] WordBreak-->>User: true/false result Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes introduce three new algorithmic implementations with distinct patterns: a sliding window (relatively straightforward), a Trie system with heap-based ranking (moderate complexity including memory management), and a Trie-DP hybrid (mixed logic across classes). While heterogeneous in approach, each is self-contained and follows conventional data structure implementations, but collectively requires reviewing distinct algorithmic reasoning and correctness for each file. Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
I) String/17.Smallest Substring.cpp (1)
70-80: Consider adding edge case tests.The current test cases cover basic scenarios, but consider testing:
- Empty strings
- Single character strings
- Cases where t has duplicate characters
- Cases where s and t are equal
U) Trie/Word Break.cpp (1)
66-86: Optimize by avoiding substring creation.The current implementation creates a new substring on line 78 for each (i, j) pair, which adds overhead. The actual complexity is O(n² * m) due to substring creation and copying, not O(n * m) as documented.
Consider checking characters directly in the Trie without creating substrings, or use string_view to avoid copies:
- if (dp[j] && trie.search(s.substr(j, i - j))) { + if (dp[j] && trie.search(string_view(s).substr(j, i - j))) {Or better yet, modify the Trie search to accept iterators or string_view to avoid any copying.
U) Trie/Search Engine Autocomplete System.cpp (1)
48-60: Consider memory optimization.Storing the complete word as a key in
wordFrequencyat the terminal node is redundant, as the word can be reconstructed from the path. For better memory efficiency, consider storing just the frequency or using a boolean flag with a separate frequency field.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
I) String/17.Smallest Substring.cpp(1 hunks)U) Trie/Search Engine Autocomplete System.cpp(1 hunks)U) Trie/Word Break.cpp(1 hunks)
🧰 Additional context used
🪛 Clang (14.0.6)
U) Trie/Search Engine Autocomplete System.cpp
[error] 20-20: 'iostream' file not found
(clang-diagnostic-error)
U) Trie/Word Break.cpp
[error] 17-17: 'iostream' file not found
(clang-diagnostic-error)
I) String/17.Smallest Substring.cpp
[error] 14-14: 'iostream' file not found
(clang-diagnostic-error)
🔇 Additional comments (3)
I) String/17.Smallest Substring.cpp (1)
21-68: LGTM! Solid sliding window implementation.The logic correctly handles frequency matching and window minimization. The algorithm efficiently finds the minimum window containing all target characters with O(n) time complexity.
U) Trie/Search Engine Autocomplete System.cpp (2)
128-145: LGTM!The updateFrequency method correctly navigates to the word's terminal node and updates the frequency if the word exists.
147-159: LGTM!The search method correctly traverses the Trie and verifies word existence at the terminal node.
| class TrieNode { | ||
| public: | ||
| unordered_map<char, TrieNode*> children; | ||
| bool isEndOfWord; | ||
| | ||
| TrieNode() { | ||
| isEndOfWord = false; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Add destructor to prevent memory leak.
The TrieNode class stores raw pointers in the children map but lacks a destructor to free them. This causes memory leaks when nodes are destroyed.
Add a destructor to recursively delete child nodes:
class TrieNode { public: unordered_map<char, TrieNode*> children; bool isEndOfWord; TrieNode() { isEndOfWord = false; } + + ~TrieNode() { + for (auto& pair : children) { + delete pair.second; + } + } };🤖 Prompt for AI Agents
In U) Trie/Word Break.cpp around lines 24 to 32, the TrieNode class owns raw child pointers but lacks a destructor, causing memory leaks; add a destructor that iterates over the children map, deletes each TrieNode*, and clears the map to free memory, and also either delete the copy constructor and copy assignment operator (or implement deep copy) to avoid double-free or dangling pointers when nodes are copied. | class Trie { | ||
| private: | ||
| TrieNode* root; | ||
| | ||
| public: | ||
| Trie() { | ||
| root = new TrieNode(); | ||
| } | ||
| | ||
| void insert(const string &word) { | ||
| TrieNode* current = root; | ||
| for (char c : word) { | ||
| if (current->children.find(c) == current->children.end()) { | ||
| current->children[c] = new TrieNode(); | ||
| } | ||
| current = current->children[c]; | ||
| } | ||
| current->isEndOfWord = true; | ||
| } | ||
| | ||
| bool search(const string &word) { | ||
| TrieNode* current = root; | ||
| for (char c : word) { | ||
| if (current->children.find(c) == current->children.end()) { | ||
| return false; | ||
| } | ||
| current = current->children[c]; | ||
| } | ||
| return current->isEndOfWord; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Add destructor to clean up the root node.
The Trie class allocates the root node but never deletes it, causing a memory leak.
Add a destructor:
class Trie { private: TrieNode* root; public: Trie() { root = new TrieNode(); } + + ~Trie() { + delete root; + } void insert(const string &word) { // ... }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In U) Trie/Word Break.cpp around lines 34 to 64, the Trie constructor allocates root but there is no destructor, causing a memory leak; add a destructor ~Trie() that frees all allocated TrieNode objects by implementing a private recursive helper (e.g., void deleteNode(TrieNode* node)) which traverses children, deletes each child node, then deletes the node itself, and call this helper on root in the destructor and set root to nullptr. 
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
U) Trie/Search Engine Autocomplete System.cpp (2)
78-107: Critical bug: autocomplete only returns words ending at prefix node, missing words that extend beyond prefix.The
autocompletemethod only examineswordFrequencyat the prefix's terminal node (line 93), butinsertstores each word at its own terminal node. Words extending beyond the prefix are stored in deeper nodes and won't be collected.Example:
- Insert "app" (freq 10) and "apple" (freq 5)
autocomplete("app", 2)navigates to the node after the second 'p', finds only "app" in itswordFrequency, and misses "apple" stored at the 'e' nodeTraverse the entire subtree:
vector<string> autocomplete(const string &prefix, int K) { TrieNode *current = root; vector<string> results; for (char c : prefix) { if (current->children.find(c) == current->children.end()) { return results; // No words with this prefix } current = current->children[c]; } priority_queue<pair<int, string>> pq; - for (const auto &entry : current->wordFrequency) - { - pq.push({entry.second, entry.first}); // Store frequency and word - } + collectAllWords(current, pq); int count = 0; while (!pq.empty() && count < K) { results.push_back(pq.top().second); pq.pop(); count++; } return results; } + + void collectAllWords(TrieNode* node, priority_queue<pair<int, string>>& pq) + { + if (!node) return; + + for (const auto &entry : node->wordFrequency) + { + pq.push({entry.second, entry.first}); + } + + for (const auto &child : node->children) + { + collectAllWords(child.second, pq); + } + }
114-142: Fix node deletion logic to check both children and wordFrequency.Lines 121 and 138 only check
children.empty()before deleting a node. A node storing other words inwordFrequencymust not be deleted even if it has no children.Apply this fix:
if (index == word.length()) { if (current->wordFrequency.find(word) != current->wordFrequency.end()) { current->wordFrequency.erase(word); - return current->children.empty(); + return current->children.empty() && current->wordFrequency.empty(); } return false; } char c = word[index]; if (current->children.find(c) == current->children.end()) { return false; // Word doesn't exist } TrieNode *nextNode = current->children[c]; bool shouldDeleteCurrentNode = deleteWordHelper(nextNode, word, index + 1); if (shouldDeleteCurrentNode) { current->children.erase(c); - return current->children.empty(); + return current->children.empty() && current->wordFrequency.empty(); } return false;
🧹 Nitpick comments (2)
U) Trie/Search Engine Autocomplete System.cpp (2)
28-35: Consider explicit copy/move deletion for TrieNode.
TrieNodeowns raw pointers in itschildrenmap but lacks copy/move control. While the current design keepsTrieNodeinternal toTrie, explicitly deleting copy constructor and copy assignment operator prevents accidental misuse.Add these declarations to prevent copying:
class TrieNode { public: unordered_map<char, TrieNode *> children; unordered_map<string, int> wordFrequency; TrieNode() {} + TrieNode(const TrieNode&) = delete; + TrieNode& operator=(const TrieNode&) = delete; };
37-51: Consider explicit copy/move deletion for Trie class.The
Trieclass manages a dynamically allocatedrootpointer but lacks copy/move control. Without these, copying aTriewould create shallow copies sharing the same root, leading to double-free on destruction.Add these to the public section:
class Trie { private: TrieNode *root; public: Trie() { root = new TrieNode(); } ~Trie() { deleteAll(root); } + + Trie(const Trie&) = delete; + Trie& operator=(const Trie&) = delete;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
U) Trie/Search Engine Autocomplete System.cpp(1 hunks)
🧰 Additional context used
🪛 Clang (14.0.6)
U) Trie/Search Engine Autocomplete System.cpp
[error] 20-20: 'iostream' file not found
(clang-diagnostic-error)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
U) Trie/Search Engine Autocomplete System.cpp (3)
28-35: Add destructor to prevent memory leak.The TrieNode class stores raw pointers in the
childrenmap but lacks a destructor, causing memory leaks when TrieNode instances are destroyed.Apply this diff:
class TrieNode { public: unordered_map<char, TrieNode *> children; unordered_map<string, int> wordFrequency; TrieNode() {} + + ~TrieNode() { + for (auto& pair : children) { + delete pair.second; + } + } };
53-62: Critical bug: incorrect iteration over unordered_map causes memory leak.The
deleteAllmethod uses integer indexing (node->children[i]) on anunordered_map<char, TrieNode*>, which doesn't iterate over actual child nodes. This creates/accesses entries with integer keys (0-25) instead of char keys, leaving real child TrieNodes unreleased and causing memory leaks.Apply this fix:
void deleteAll(TrieNode* node) { if (!node) return; - // Recursively delete all children - for (int i = 0; i < 26; i++) { // Assuming 26 letters a-z - if (node->children[i]) { - deleteAll(node->children[i]); - } - } + for (auto& pair : node->children) { + deleteAll(pair.second); + } delete node; }
128-156: Fix node deletion logic to check wordFrequency.Lines 135 and 152 only check if
childrenis empty before deciding to delete a node. However, a node may have other words stored in itswordFrequencymap. It shouldn't be deleted even if it has no children when other words remain.Apply this fix:
if (index == word.length()) { if (current->wordFrequency.find(word) != current->wordFrequency.end()) { current->wordFrequency.erase(word); - return current->children.empty(); + return current->children.empty() && current->wordFrequency.empty(); } return false; } char c = word[index]; if (current->children.find(c) == current->children.end()) { return false; } TrieNode *nextNode = current->children[c]; bool shouldDeleteCurrentNode = deleteWordHelper(nextNode, word, index + 1); if (shouldDeleteCurrentNode) { current->children.erase(c); - return current->children.empty(); + return current->children.empty() && current->wordFrequency.empty(); } return false;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
U) Trie/Search Engine Autocomplete System.cpp(1 hunks)
🧰 Additional context used
🪛 Clang (14.0.6)
U) Trie/Search Engine Autocomplete System.cpp
[error] 20-20: 'iostream' file not found
(clang-diagnostic-error)
🔇 Additional comments (3)
U) Trie/Search Engine Autocomplete System.cpp (3)
78-121: LGTM: Autocomplete subtree traversal fixed.The autocomplete method now correctly traverses the entire subtree rooted at the prefix node via
collectAllWords, addressing the critical bug from the previous review where it only checked the prefix node'swordFrequency.
158-175: LGTM: Update frequency logic is correct.The method properly traverses the word path, verifies the word exists in the terminal node's
wordFrequencymap, and updates the frequency accordingly.
177-189: LGTM: Search logic is correct.The method correctly traverses the word path and verifies the word exists in the terminal node's
wordFrequencymap, ensuring it doesn't return true for mere prefixes.
| private: | ||
| void deleteAll(TrieNode* node) { | ||
| if (!node) return; | ||
| // Recursively delete all children | ||
| for (int i = 0; i < 26; i++) { // Assuming 26 letters a-z | ||
| if (node->children[i]) { | ||
| deleteAll(node->children[i]); | ||
| } | ||
| } | ||
| delete node; | ||
| } | ||
| |
There was a problem hiding this comment.
Critical bug: missing public: access specifier makes all API methods private.
After the private: label at line 52, all subsequent methods (insert, autocomplete, deleteWord, updateFrequency, search) are private. However, main() calls these methods at lines 196, 202, 209, 212, and 223. This code should not compile.
Apply this fix to add a public: section after deleteAll:
void deleteAll(TrieNode* node) { if (!node) return; // Recursively delete all children for (int i = 0; i < 26; i++) { // Assuming 26 letters a-z if (node->children[i]) { deleteAll(node->children[i]); } } delete node; } + +public: void insert(const string &word, int frequency) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private: | |
| void deleteAll(TrieNode* node) { | |
| if (!node) return; | |
| // Recursively delete all children | |
| for (int i = 0; i < 26; i++) { // Assuming 26 letters a-z | |
| if (node->children[i]) { | |
| deleteAll(node->children[i]); | |
| } | |
| } | |
| delete node; | |
| } | |
| private: | |
| void deleteAll(TrieNode* node) { | |
| if (!node) return; | |
| // Recursively delete all children | |
| for (int i = 0; i < 26; i++) { // Assuming 26 letters a-z | |
| if (node->children[i]) { | |
| deleteAll(node->children[i]); | |
| } | |
| } | |
| delete node; | |
| } | |
| public: | |
| void insert(const string &word, int frequency) | |
| { |
🤖 Prompt for AI Agents
In U) Trie/Search Engine Autocomplete System.cpp around lines 52 to 63, the class currently ends its private section with deleteAll, causing subsequent API methods (insert, autocomplete, deleteWord, updateFrequency, search) to remain private and break calls from main; add a public: access specifier immediately after the deleteAll method (after the closing brace of deleteAll and before the next method declarations) so the public API methods are accessible to main. Added destructor to TrieNode for proper memory management and updated delete logic to consider word frequency.
AnkitMajee added enhancement 
| @shubhamranswal Great work 👍 |
Thanks for the appreciation @AnkitMajee |
2 participants
Hi @AnkitMajee,
I’ve added a few questions that are commonly asked in SWE - Intuit interviews. I hope they will be helpful for others who use this repo. Thanks for maintaining this awesome resource!
Below are the problem descriptions:
Search Engine Autocomplete System:
mis the length of the input query, ensuring quick and efficient retrieval of suggestions.Word Break using Trie and DP:
nis the length of the string andmis the maximum word length.Smallest Substring Containing All Characters:
sthat contains all characters from stringt.nis the length of strings.Let me know if there’s anything else you’d like me to change. Thanks again for this repo!
Summary by CodeRabbit