Skip to content
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

[FEATURE]: More explicit handling in QuickSort partition algorithm #232

Open
zsh-eng opened this issue Mar 2, 2024 · 1 comment
Open
Labels
enhancement New feature or request

Comments

@zsh-eng
Copy link

zsh-eng commented Mar 2, 2024

Motivation

I've been reviewing the Quicksort partition implementation in this repository. Specifically, I'm focusing on the following segment of the code:

while (i < j) {
 while (array[++i] < pivot);
 while (array[--j] > pivot);

 if (i < j) {
    [array[i], array[j]] = [array[j], array[i]];
 }
}

In the scenario where the rightmost element is chosen as the pivot, and all other elements are greater than the pivot (e.g., [6, 5, 4, 3, 2, 1]), the code handles the out-of-bounds behavior as follows: array[-1] is undefined. When undefined is compared against a number, it is coerced into NaN, resulting in false for the comparison.

While this approach works and is quite clever, I believe it could be beneficial to make the handling of these cases more explicit. This would enhance the readability and maintainability of the code, making it easier for newcomers to understand and for the code to be adapted or modified in the future.

I suggest taking a more conventional approach, such as:

while (array[--j] > pivot) {
 if (j === left) break;
}

This change would make the algorithm's logic more straightforward and easier to follow, without relying on special JavaScript behavior.

Thank you for considering this request. I'm open to feedback and suggestions on how to further improve the code.

Examples

No response

Possible workarounds

No response

@zsh-eng zsh-eng added the enhancement New feature or request label Mar 2, 2024
@appgurueu
Copy link
Contributor

I agree that relying on the type coercion done by > here is hacky; it should be made explicit.

Why not do something like while (array[--j] > pivot && j !== left); though?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants