feat/(lgorithms, backtracking dfs trie): word search#130
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis PR moves Word Search from Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Test as Test Runner
participant Finder as find_strings
participant Grid as Grid (2D array)
participant Trie as Trie
Note over Test,Finder: Start multi-word search
Test->>Finder: find_strings(grid, words)
Finder->>Trie: build trie(words)
Finder->>Grid: iterate cells (i,j)
alt cell starts a trie path
Finder->>Trie: prefix lookup (char)
Finder->>Grid: DFS explore neighbors (up/down/left/right)
Grid->>Finder: char at coordinate
Finder->>Trie: advance node
alt word found
Finder->>Trie: mark found, call remove_characters(word)
Finder->>Finder: record word in results
end
end
Finder-->>Test: return found words list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 2 inconclusive)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
datastructures/trees/trie/trie.py (1)
88-106: Missing return type annotation and potential KeyError if string doesn't exist.The method lacks a return type annotation and will raise
KeyErrorifstring_to_deletedoesn't exist in the trie. While current usage infind_stringsensures the path exists, defensive coding would improve robustness.🔎 Proposed improvements
- def remove_characters(self, string_to_delete: str): + def remove_characters(self, string_to_delete: str) -> None: """ Removes a string from the trie """ node = self.root child_list = [] for c in string_to_delete: + if c not in node.children: + return # String not in trie, nothing to remove - child_list.append([node, c]) + child_list.append((node, c)) node = node.children[c] for pair in reversed(child_list): parent = pair[0] child_char = pair[1] target = parent.children[child_char] if target.children: return del parent.children[child_char]algorithms/backtracking/word_search/README.md (1)
9-20: Missing language specifier for fenced code block.The code block at line 9 should have a language specifier for proper syntax highlighting.
🔎 Proposed fix
-``` +```text jefblpepre camdcimgtc oivokprjsm pbwasqroua rixilelhrs wolcqlirpc screeaumgr alxhpburyi jalaycalmp clojurermt</details> </blockquote></details> <details> <summary>algorithms/backtracking/word_search/point.py (2)</summary><blockquote> `7-14`: **Consider adding type hints for constructor parameters.** The `Point` class would benefit from type annotations for consistency with the rest of the codebase. <details> <summary>🔎 Proposed improvement</summary> ```diff - def __init__(self, x, y): + def __init__(self, x: int, y: int) -> None: """ Creates a new cartesian point object :param x: point on x-axis :param y: point on y-axis """ self.x = x self.y = y
16-17: Non-standard__repr__format uses colon separator.The
__repr__usesPoint(x:y)format instead of the conventionalPoint(x, y). This doesn't affect functionality but could be confusing since it doesn't match the constructor signature.🔎 Proposed fix
def __repr__(self): - return "Point({}:{})".format(self.x, self.y) + return f"Point({self.x}, {self.y})"algorithms/backtracking/word_search/__init__.py (1)
92-93: Consider using a named function instead of lambda assignment.Assigning a lambda to a variable is discouraged by PEP 8 (E731). A named function provides better debugging and documentation.
🔎 Proposed refactor
- # lambda function to check if the current cell is within the grid - is_cell_within_grid = lambda r, c: 0 <= r < rows_count and 0 <= c < cols_count + def is_cell_within_grid(r: int, c: int) -> bool: + """Check if the current cell is within the grid.""" + return 0 <= r < rows_count and 0 <= c < cols_count
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
algorithms/backtracking/word_search/images/examples/word_search_two_example_1.pngis excluded by!**/*.pngalgorithms/backtracking/word_search/images/examples/word_search_two_example_2.pngis excluded by!**/*.pngalgorithms/backtracking/word_search/images/examples/word_search_two_example_3.pngis excluded by!**/*.pngalgorithms/backtracking/word_search/images/examples/word_search_two_example_4.pngis excluded by!**/*.pngalgorithms/backtracking/word_search/images/examples/word_search_two_example_5.pngis excluded by!**/*.png
📒 Files selected for processing (11)
DIRECTORY.mdalgorithms/backtracking/word_search/README.mdalgorithms/backtracking/word_search/__init__.pyalgorithms/backtracking/word_search/constants.pyalgorithms/backtracking/word_search/point.pyalgorithms/backtracking/word_search/test_word_search.pyalgorithms/backtracking/word_search/test_word_search_two.pyalgorithms/word_count/test_word_count.pyalgorithms/word_search/README.mdalgorithms/word_search/__init__.pydatastructures/trees/trie/trie.py
💤 Files with no reviewable changes (2)
- algorithms/word_search/README.md
- algorithms/word_search/init.py
🧰 Additional context used
🧬 Code graph analysis (4)
algorithms/backtracking/word_search/__init__.py (2)
algorithms/backtracking/word_search/point.py (1)
Point(1-29)datastructures/trees/trie/trie.py (5)
Trie(5-109)search(51-73)insert(9-49)dfs(65-70)remove_characters(88-106)
algorithms/backtracking/word_search/test_word_search_two.py (1)
algorithms/backtracking/word_search/__init__.py (1)
find_strings(71-147)
algorithms/backtracking/word_search/test_word_search.py (2)
algorithms/backtracking/word_search/point.py (1)
Point(1-29)algorithms/backtracking/word_search/__init__.py (1)
WordSearch(8-68)
algorithms/backtracking/word_search/constants.py (1)
algorithms/backtracking/word_search/point.py (1)
Point(1-29)
🪛 LanguageTool
algorithms/backtracking/word_search/README.md
[grammar] ~32-~32: Ensure spelling is correct
Context: ...this word. It must return the Points of thw first and last letter of the word if fo...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~54-~54: Ensure spelling is correct
Context: ...d the last letter e can be found at poin 6, 9 > Note: indexes start counting fro...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~77-~77: To elevate your writing, try using an alternative expression here.
Context: ... The order of the strings in the output does not matter. ## Examples 
🪛 markdownlint-cli2 (0.18.1)
DIRECTORY.md
41-41: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
42-42: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
43-43: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
44-44: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
45-45: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
182-182: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
183-183: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
algorithms/backtracking/word_search/README.md
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
73-73: Reference links and images should use a label that is defined
Missing link or image reference definition: "j"
(MD052, reference-links-images)
105-105: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
🔇 Additional comments (5)
algorithms/backtracking/word_search/test_word_search.py (1)
2-3: LGTM!The import paths are correctly updated to reflect the module relocation to
algorithms/backtracking/word_search. The test coverage for various word search directions (horizontal, vertical, diagonal, and edge cases) is comprehensive.DIRECTORY.md (1)
41-45: LGTM!The new Word Search entries under the Backtracking section correctly document the relocated and newly added files (constants.py, point.py, and test files). The structure matches the existing documentation format.
algorithms/backtracking/word_search/constants.py (1)
1-13: LGTM!The
PLANE_LIMITSconstant correctly defines all eight directional offsets (cardinal and diagonal) for grid traversal. Using an immutable tuple is appropriate for a constant, and thePointobjects provide consistent vector arithmetic throughout the word search implementation.algorithms/backtracking/word_search/test_word_search_two.py (1)
64-70: LGTM! Good use of parameterized testing.The test structure is clean with comprehensive test cases covering various grid configurations. Consider adding edge case tests for empty grids or empty word lists in a future iteration.
algorithms/backtracking/word_search/__init__.py (1)
71-147: Well-implemented Word Search 2 algorithm using Trie with DFS backtracking.The implementation correctly:
- Builds a Trie from input words for efficient prefix matching
- Uses DFS with backtracking to explore all paths
- Prevents duplicates by clearing
is_endafter finding a word- Prunes the Trie to optimize future searches
- Properly manages the visited set for backtracking
The algorithm aligns with the documented O(n*3^l) time complexity.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Describe your change:
Word search
Checklist:
Fixes: #{$ISSUE_NO}.Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.