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

fix: Exercise10_FasterMerge.java #243

Closed
wants to merge 2 commits into from

Conversation

XinXiaoIsMe
Copy link

I run your code and find it should be indexRight but not middle, this is a little mistake but confused me some time. I left a comment in issue#21 but later I think it's better to open a pull request. Thank you again for your contribution.

@reneargento
Copy link
Owner

I ran a few tests and using middle instead of indexRight seems to give correct results. Using indexRight also gives correct results, but we do extra comparisons that can be avoided.
I believe it is because when we reach the middle index in the loop, all elements between middle + 1 and indexRight are higher than the elements between low and middle (as they are sorted in a decreasing order). So they are already in the correct position in the main array and we can stop the loop.
If you find a counter case where using middle instead of indexRight gives an incorrect result let me know.

@reneargento reneargento mentioned this pull request Mar 20, 2022
@XinXiaoIsMe
Copy link
Author

I ran a few tests and using middle instead of indexRight seems to give correct results. Using indexRight also gives correct results, but we do extra comparisons that can be avoided. I believe it is because when we reach the middle index in the loop, all elements between middle + 1 and indexRight are higher than the elements between low and middle (as they are sorted in a decreasing order). So they are already in the correct position in the main array and we can stop the loop. If you find a counter case where using middle instead of indexRight gives an incorrect result let me know.

You are right! I am too careless. Thanks for your patience.

@yetanothercoder
Copy link

yetanothercoder commented May 29, 2022

no sense to compare after leftIndex reaches the middle: this mean that up to this point of time - all items from sorted left half are less than sorted right half, so the merging is not needed anymore.

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.

3 participants