feat(algorithms hash table): ransom note#160
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a new "Ransom Note" hash-table algorithm (docs, two implementations, tests), updates circular linked list type hints to use Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
datastructures/linked_lists/linked_list_utils.py (1)
150-173: Validatento avoid runtime errors for out‑of‑range inputs.
n > lengthcurrently raises an AttributeError, andn <= 0removes the tail unintentionally. Add explicit bounds checks to make the utility safe.🛠️ Proposed fix
def remove_nth_from_end(head: Optional[Node], n: int) -> Optional[Node]: @@ if not head: return head + if n <= 0: + raise ValueError("n must be >= 1") @@ - for _ in range(n): - fast = fast.next + for _ in range(n): + if not fast: + raise ValueError("n is larger than the list length") + fast = fast.nextdatastructures/linked_lists/circular/__init__.py (1)
201-236:split_listreturn type/behavior is inconsistent with the docstring.The method returns
(self.head, second_list)even though the docstring describes two circular linked lists; the type hint also suggests nodes whilesecond_listis aCircularLinkedList. This mismatch can break callers and tests. Return list objects instead (and update tests accordingly).🛠️ Proposed fix
- def split_list(self) -> Optional[Tuple[CircularNode, Optional[CircularNode]]]: + def split_list( + self, + ) -> Optional[Tuple["CircularLinkedList", Optional["CircularLinkedList"]]]: @@ - if size == 1: - return self.head, None + if size == 1: + return self, None @@ - return self.head, second_list + return self, second_list
🤖 Fix all issues with AI agents
In `@DIRECTORY.md`:
- Around line 160-162: The Markdown list indentation for the "Hash Table"
section is off for MD007: change the three-level block so the top-level bullet
"* Hash Table" starts at 0 spaces, the nested "* Ransom Note" at 2 spaces, and
the link item "[Test Ransom Note](...)" at 4 spaces; update those three lines in
DIRECTORY.md accordingly to follow the 0/2/4 indentation convention.
🧹 Nitpick comments (2)
algorithms/hash_table/ransom_note/__init__.py (1)
32-51: Simplifycan_constructby dropping the redundantcount.You can return
Trueafter the loop since any failure already returnsFalse. This reduces state and keeps the logic tight.♻️ Proposed simplification
- # Count the number of letters in the ransom note. This will be used to track how many letters are left when constructing - # the ransom note from the letters in the magazine - count = len(ransom_note) @@ - occurrences[letter] -= 1 - count -= 1 + occurrences[letter] -= 1 @@ - return count == 0 + return Truealgorithms/hash_table/ransom_note/test_ransom_note.py (1)
5-14: Consider adding edge case tests and named parameters for better diagnostics.The test cases cover core scenarios well, but consider:
- Adding descriptive names to test cases for clearer failure messages
- Adding edge cases for empty strings (empty ransom_note, empty magazine)
Note: The two implementations behave differently for edge cases—
can_constructreturnsFalsefor an empty magazine even when ransom_note is empty, whilecan_construct_2returnsTruefor an empty ransom_note. You may want to align this behavior and add explicit tests.♻️ Suggested improvements
RANSOM_NOTE_TEST_CASES = [ - ("codinginterviewquestions", "aboincsdefoetingvqtniewonoregessnutins", True), - ("code", "coingd", False), - ("codinginterview", "vieewidingcodinter", True), - ("program", "programming", True), - ("me", "meme", True), - ("a", "b", False), - ("aa", "ab", False), - ("aa", "aab", True), + ("long_note_success", "codinginterviewquestions", "aboincsdefoetingvqtniewonoregessnutins", True), + ("missing_letter", "code", "coingd", False), + ("shuffled_letters", "codinginterview", "vieewidingcodinter", True), + ("subset_of_magazine", "program", "programming", True), + ("repeated_letters", "me", "meme", True), + ("single_char_mismatch", "a", "b", False), + ("insufficient_repeated_char", "aa", "ab", False), + ("sufficient_repeated_char", "aa", "aab", True), + ("empty_ransom_note", "", "abc", True), + ("empty_magazine", "a", "", False), ]
Describe your change:
Ransom note implementation with a Hash table
Checklist:
Fixes: #{$ISSUE_NO}.Summary by CodeRabbit
New Features
Documentation
Tests
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.