-
Notifications
You must be signed in to change notification settings - Fork 2
feat(algorithms, dynamic programming): maximal square #157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(algorithms, dynamic programming): maximal square #157
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new Maximal Square dynamic programming solution with documentation and test coverage, while enhancing the LinkedList class with improved cycle detection capabilities including a new cycle_length method and updated type annotations for detect_node_with_cycle. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
datastructures/linked_lists/__init__.py (1)
439-442: Critical bug: Method mutatesself.headas a side effect.Lines 439-441 modify
self.headwhile traversing to find the cycle start. After this method returns, the linked list's head will point to the cycle start node instead of the original head, corrupting the list's state.🐛 Proposed fix: Use a temporary variable instead of modifying self.head
- while self.head != slow_pointer: - slow_pointer = slow_pointer.next - self.head = self.head.next - return self.head + head_pointer = self.head + while head_pointer != slow_pointer: + slow_pointer = slow_pointer.next + head_pointer = head_pointer.next + return head_pointer
🧹 Nitpick comments (3)
datastructures/linked_lists/__init__.py (1)
425-435: Consider avoiding redundant cycle detection.The method runs Floyd's algorithm twice: once via
has_cycle()on line 425, and again in the loop on lines 430-435. You could combine these into a single pass for better efficiency.♻️ Optional refactor to eliminate redundant traversal
def detect_node_with_cycle(self) -> Optional[Node]: """ Detects the node with a cycle and returns it """ - if not self.has_cycle(): - return None - else: - slow_pointer = fast_pointer = self.head + slow_pointer = fast_pointer = self.head - while fast_pointer and slow_pointer and fast_pointer.next: - fast_pointer = fast_pointer.next.next - slow_pointer = slow_pointer.next + while fast_pointer and fast_pointer.next: + fast_pointer = fast_pointer.next.next + slow_pointer = slow_pointer.next - if slow_pointer == fast_pointer: - break - else: - return None + if slow_pointer == fast_pointer: + # Cycle detected, find the start node + head_pointer = self.head + while head_pointer != slow_pointer: + slow_pointer = slow_pointer.next + head_pointer = head_pointer.next + return head_pointer - while self.head != slow_pointer: - slow_pointer = slow_pointer.next - self.head = self.head.next - return self.head + return Nonealgorithms/dynamic_programming/maximal_square/README.md (1)
23-32: Use inline code formatting for variable references.The array indexing notation (e.g.,
dp[i][j],matrix[i - 1][j - 1]) in prose text is being interpreted as Markdown reference links. Wrap these in backticks to render them as inline code and avoid linting warnings.📝 Suggested formatting fix
-We create a 2D integer array dp of size (r + 1) x (c + 1) where r is the number of rows in the input array and c is the -size of each row. dp[i][j] stores the side length of the largest square ending at the cell matrix[i - 1][j - 1]. All +We create a 2D integer array `dp` of size `(r + 1) x (c + 1)` where `r` is the number of rows in the input array and `c` is the +size of each row. `dp[i][j]` stores the side length of the largest square ending at the cell `matrix[i - 1][j - 1]`. All elements of dp are initialized to 0.-We then use a nested loop to iterate over the input array. For each cell matrix[i - 1][j - 1], we check if the cell -contains a 1. If it does, we update dp[i][j] to the minimum of the sizes of the largest squares ending at the three +We then use a nested loop to iterate over the input array. For each cell `matrix[i - 1][j - 1]`, we check if the cell +contains a 1. If it does, we update `dp[i][j]` to the minimum of the sizes of the largest squares ending at the three adjacent cells plus 1.algorithms/dynamic_programming/maximal_square/test_maximal_square.py (1)
6-18: Test cases are correct. Consider adding edge cases.The parameterized test structure is clean. The test cases correctly verify the expected outputs. Consider expanding coverage with:
- Empty matrix
[]→ expected0- Single cell with
1:[[1]]→ expected1- Larger square (e.g., 3×3 all ones) to verify scaling
📝 Suggested additional test cases
MAXIMAL_SQUARE_TEST_CASES = [ ( [ [1, 0, 1, 0, 0], [1, 0, 1, 1, 1], [1, 1, 1, 1, 1], [1, 0, 0, 1, 0], ], 4, ), ([[0, 1], [1, 0]], 1), ([[0]], 0), + ([], 0), # empty matrix + ([[1]], 1), # single cell with 1 + ([[1, 1, 1], [1, 1, 1], [1, 1, 1]], 9), # 3x3 all ones ]
Describe your change:
Maximal square algorithm to find the maximum area of a square that only contains 1
Checklist:
Fixes: #{$ISSUE_NO}.Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.