Skip to content

Optimize Quick Sort, N-Sum, and fix bug in Is Palindrome#2745

Open
DhruvrajSinhZala24 wants to merge 2 commits into
keon:mainfrom
DhruvrajSinhZala24:main
Open

Optimize Quick Sort, N-Sum, and fix bug in Is Palindrome#2745
DhruvrajSinhZala24 wants to merge 2 commits into
keon:mainfrom
DhruvrajSinhZala24:main

Conversation

@DhruvrajSinhZala24

Copy link
Copy Markdown
  1. Quick Sort: Added Median-of-Three pivot selection and Insertion Sort for small subarrays.\n2. N-Sum: Refactored to reduce redundant sorts and copies using recursion with indices.\n3. Is Palindrome: Fixed potential IndexError and improved efficiency.

1. Quick Sort: Added Median-of-Three pivot selection and Insertion Sort for small subarrays.\n2. N-Sum: Refactored to reduce redundant sorts and copies using recursion with indices.\n3. Is Palindrome: Fixed potential IndexError and improved efficiency.
…thms

Optimize Quick Sort, N-Sum, and fix bug in Is Palindrome

@keon keon left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified locally: test_array.py, test_sorting.py, and test_string.py all pass (116/116). Algorithmically the changes look fine — median-of-three + insertion-sort threshold for quicksort is a standard, well-motivated optimization, and the n-sum refactor to indices rather than slicing is a real improvement.

A few process notes for the maintainer:

  1. Bundling. Three unrelated changes (quicksort optimization, n-sum refactor, is_palindrome bugfix) in a single PR makes review and revert harder. Splitting into three PRs would be much friendlier to review.

  2. Overlap with #2749. The is_palindrome portion duplicates #2749, which is a tighter, more focused fix to the same root cause (_remove_punctuation stripping digits). I'd suggest landing #2749 first and dropping the is_palindrome changes from this PR.

  3. n-sum behavior. The refactor drops the sorted([...]) call on each appended tuple that the original _append_elem_to_each_list had. The new code relies on the input being sorted (which it is, via the upstream nums.sort()) so output order is preserved — that's correct, but worth a one-line comment in the code to flag the invariant for future readers.

Not blocking — just suggesting a cleaner split.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants