- Notifications
You must be signed in to change notification settings - Fork932
Update heap.js#114
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Update heap.js#114
Conversation
Fix proper parent index fetching
Hi@Balaji1202! Thanks a lot for the fix. Could you please add a unit test that captures this bug fix? |
Hey@amejiarosario! The issue seems to be a little complicated and the ceil operation is little weird from javascript side too. The operation But, for an input of 1, the javascript engine equates Math.ceil(-0.5) to negative zero So, I feel my fix may not add any value since there is not any issue in first place. But, if you are worried about reading confusion for your readers with the priority of the execution in the statement, think about merging it. Else feel free to close the PR. :>) |
Ah, I see now where you are coming from. I agree this change makes it easier to reason about. Also, it avoids the weird |
Fix proper parent index fetching