-
Notifications
You must be signed in to change notification settings - Fork 1
344: reverse string #111
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
base: main
Are you sure you want to change the base?
344: reverse string #111
Conversation
WalkthroughNew LeetCode reverse string implementation added with two-pointer swap approach. Top-k frequent elements algorithms introduced using both sorting and heap-based methods in array hashing solutions. Test suite updated with formatting normalization and new reverse string test case. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (4)
LeetCode/easy/reverse_string_344.py (1)
6-18: Reverse logic is correct; swap helper could be inlinedThe two‑pointer in‑place reversal is correct and handles empty, odd, and even‑length inputs as expected. If you want to simplify, you can drop the
swaphelper and use direct tuple unpacking:while left < right: s[left], s[right] = s[right], s[left] left += 1 right -= 1Current version is fine if you prefer the explicit helper.
tests/test_leetcode_easy.py (1)
94-116: Reverse-string tests look good; consider adding an empty-input caseThe parametrized
test_reverse_string_344correctly exercises in‑place mutation for a variety of lengths and character patterns and matches the LeetCode contract of not using the return value.To tighten coverage, you might add a case for an empty list (and optionally a single‑character list), e.g.
([], []), to pin boundary behavior.algorithms/python/array_hashing.py (2)
1-2: topKFrequent implementations are correct; a few simplifications are possibleBoth
topKFrequentandtopKFrequentHeapcorrectly compute the k most frequent elements and are fine for typical LeetCode constraints.If you want to streamline a bit:
For
topKFrequent, you can avoid constructing an intermediate dict and just sort keys by frequency:counter = {} for num in nums: counter[num] = counter.get(num, 0) + 1 return sorted(counter, key=counter.get, reverse=True)[:k]For the heap version, behavior is correct; just note that the result order reflects heap order, which is fine for problems that allow “any order” but might be surprising in other contexts. A short docstring could clarify that.
These are optional polish; the current code is valid.
Also applies to: 34-60
63-66: Avoid executing demo code at import timeThe bottom snippet that builds
nums, callstopKFrequentHeap, and prints the result will run on every import of this module, which is undesirable in library/tested code and can produce unexpected stdout noise.Consider either removing this block or guarding it:
if __name__ == "__main__": nums = [3, 0, 1, 0] k = 1 sol = Solution().topKFrequentHeap(nums=nums, k=k) print("topKFrequent:", sol)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
LeetCode/easy/reverse_string_344.py(1 hunks)algorithms/python/array_hashing.py(2 hunks)tests/test_leetcode_easy.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/test_leetcode_easy.py (1)
LeetCode/easy/reverse_string_344.py (2)
Solution(6-18)reverseString(10-18)
🔇 Additional comments (1)
tests/test_leetcode_easy.py (1)
8-27: Merge test case updates are purely formattingThe
test_merge_sorted_array_88parameter values remain the same; only spacing/formatting changed. No behavioral impact on the tests.
Summary by CodeRabbit
New Features
Tests