refactor(data-structures, linked-lists): pairwise swap #138
Conversation
BrianLusina
added
enhancement
Algorithm
📝 WalkthroughWalkthroughMethods for swapping adjacent nodes in a singly linked list are refactored: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (13)
📒 Files selected for processing (3)
🧰 Additional context used🧬 Code graph analysis (2)datastructures/linked_lists/singly_linked_list/test_singly_linked_list_pairwise_swap.py (2)
datastructures/linked_lists/singly_linked_list/single_linked_list.py (2)
🪛 LanguageTooldatastructures/linked_lists/singly_linked_list/README.md[style] ~640-~640: ‘overall structure’ might be wordy. Consider a shorter alternative. (EN_WORDINESS_PREMIUM_OVERALL_STRUCTURE) 🔇 Additional comments (3)
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: 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/singly_linked_list/single_linked_list.py (1)
757-793: Critical:self.headis not updated to the new head after swapping.The implementation modifies
self.headduring the loop (line 791), but after the loop completes,self.headpoints to the node after the last swapped pair (orNone), not the actual head of the swapped list. This leaves theSinglyLinkedListinstance in an inconsistent state.The method returns the correct new head (
dummy_node.next), butself.headis not updated, which will cause subsequent operations on the linked list to fail or behave unexpectedly.🔎 Proposed fix
Use a local variable
currentinstead of mutatingself.headduring iteration, then updateself.headat the end:def pairwise_swap(self) -> Optional[SingleNode]: """ Swaps nodes in pairs without swapping the values in the nodes. Return: SingleNode: new head node or None if no head exists. """ # nothing to do here if not self.head: return self.head # Create a dummy node with a value of None dummy_node = SingleNode(None) # This dummy node acts as the prev_node for the head node # of the list and hence stores pointer to the head node dummy_node.next = self.head prev_node = dummy_node + current = self.head # While the head node and the next node exist - while self.head and self.head.next: + while current and current.next: # nodes to be swapped - first_node = self.head - second_node = self.head.next + first_node = current + second_node = current.next # perform swapping of nodes # i. Set the previous node's next node to be the second node prev_node.next = second_node # ii. Set the first node's next node to be the second node's next node first_node.next = second_node.next # iii. Set the second node's next node to be the first node second_node.next = first_node # re-initialise the head and the previous node for next swap prev_node = first_node - self.head = first_node.next + current = first_node.next + # Update the instance head to the new head + self.head = dummy_node.next return dummy_node.next
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
datastructures/linked_lists/singly_linked_list/images/solutions/singly_linked_list_swap_nodes_in_pairs_solution_1.pngis excluded by!**/*.pngdatastructures/linked_lists/singly_linked_list/images/solutions/singly_linked_list_swap_nodes_in_pairs_solution_10.pngis excluded by!**/*.pngdatastructures/linked_lists/singly_linked_list/images/solutions/singly_linked_list_swap_nodes_in_pairs_solution_11.pngis excluded by!**/*.pngdatastructures/linked_lists/singly_linked_list/images/solutions/singly_linked_list_swap_nodes_in_pairs_solution_12.pngis excluded by!**/*.pngdatastructures/linked_lists/singly_linked_list/images/solutions/singly_linked_list_swap_nodes_in_pairs_solution_13.pngis excluded by!**/*.pngdatastructures/linked_lists/singly_linked_list/images/solutions/singly_linked_list_swap_nodes_in_pairs_solution_2.pngis excluded by!**/*.pngdatastructures/linked_lists/singly_linked_list/images/solutions/singly_linked_list_swap_nodes_in_pairs_solution_3.pngis excluded by!**/*.pngdatastructures/linked_lists/singly_linked_list/images/solutions/singly_linked_list_swap_nodes_in_pairs_solution_4.pngis excluded by!**/*.pngdatastructures/linked_lists/singly_linked_list/images/solutions/singly_linked_list_swap_nodes_in_pairs_solution_5.pngis excluded by!**/*.pngdatastructures/linked_lists/singly_linked_list/images/solutions/singly_linked_list_swap_nodes_in_pairs_solution_6.pngis excluded by!**/*.pngdatastructures/linked_lists/singly_linked_list/images/solutions/singly_linked_list_swap_nodes_in_pairs_solution_7.pngis excluded by!**/*.pngdatastructures/linked_lists/singly_linked_list/images/solutions/singly_linked_list_swap_nodes_in_pairs_solution_8.pngis excluded by!**/*.pngdatastructures/linked_lists/singly_linked_list/images/solutions/singly_linked_list_swap_nodes_in_pairs_solution_9.pngis excluded by!**/*.png
📒 Files selected for processing (3)
datastructures/linked_lists/singly_linked_list/README.mddatastructures/linked_lists/singly_linked_list/single_linked_list.pydatastructures/linked_lists/singly_linked_list/test_singly_linked_list_pairwise_swap.py
🧰 Additional context used
🧬 Code graph analysis (2)
datastructures/linked_lists/singly_linked_list/test_singly_linked_list_pairwise_swap.py (2)
datastructures/linked_lists/singly_linked_list/single_linked_list.py (1)
pairwise_swap(757-793)datastructures/linked_lists/singly_linked_list/node.py (1)
SingleNode(4-10)
datastructures/linked_lists/singly_linked_list/single_linked_list.py (2)
datastructures/linked_lists/singly_linked_list/node.py (1)
SingleNode(4-10)datastructures/linked_lists/__init__.py (1)
pairwise_swap(521-534)
🪛 LanguageTool
datastructures/linked_lists/singly_linked_list/README.md
[style] ~640-~640: ‘overall structure’ might be wordy. Consider a shorter alternative.
Context: ...arrange the nodes while preserving the overall structure of the linked list. By adjusting the po...
(EN_WORDINESS_PREMIUM_OVERALL_STRUCTURE)
🔇 Additional comments (3)
datastructures/linked_lists/singly_linked_list/README.md (1)
622-685: Comprehensive documentation for the pairwise swap algorithm.The documentation is well-structured with clear problem statement, constraints, solution approach, algorithm steps, visual diagrams, and complexity analysis.
Note: The AI summary mentions this section "appears twice in the diff (duplicate insertion)." If there is indeed a duplicate, please remove one instance to avoid redundancy.
datastructures/linked_lists/singly_linked_list/test_singly_linked_list_pairwise_swap.py (1)
20-20: Test updates correctly reflect the new API.The tests have been properly updated to call the renamed
pairwise_swap()method and verify the returned head node for various cases (empty list, even-length list, and odd-length list).Also applies to: 26-26, 35-35, 39-39
datastructures/linked_lists/singly_linked_list/single_linked_list.py (1)
727-755: Good refactoring - method name now clearly indicates it modifies node values.The rename from
pairwise_swap()topairwise_swap_with_modification()makes it explicit that this method swaps node values rather than restructuring the node pointers.
Describe your change:
Pairwise swap two nodes in a linked list in place
Checklist:
Fixes: #{$ISSUE_NO}.Summary by CodeRabbit
Documentation
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.