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

btree: update value to null, after insert #93

Closed
wants to merge 1 commit into from
Closed

btree: update value to null, after insert #93

wants to merge 1 commit into from

Conversation

Horki
Copy link

@Horki Horki commented Jul 31, 2020

#92 fix for updating value to null

@kevin-wayne
Copy link
Owner

Thanks. Does this affect correctness? The documentation says that only the key and next fields of internal nodes are used. But still probably worth nulling out to prevent loitering.

@Horki
Copy link
Author

Horki commented Aug 1, 2020

My fix doesn't have a negative effect on correctness, I even wrote some simple unit tests for validation

@kevin-wayne
Copy link
Owner

Right, just confirming that it has no positive effects on correctness either. It's a stylistic improvement and fixes a loitering (memory) bug.

@Horki
Copy link
Author

Horki commented Aug 2, 2020

@kevin-wayne feel free to close it, but I taught the fix was helpful

@Horki Horki closed this Aug 3, 2020
@Horki
Copy link
Author

Horki commented Aug 3, 2020

@kevin-wayne my commit didn't remove a "final" keyword from "val", but I was aware of that, and it's really unfair from you that made the update, without giving me any kudos for finding a bug.

From that, I can only assume, if I find another bug in algs4 code, I wouldn't report it furthermore.

Kind regards,
Ivan Zvonimir Horvat

PS
You should be really ashamed

@kevin-wayne
Copy link
Owner

I use a local system to commit all changes (which predates this git repo and enables me to publish to the booksite, syntax highlight, add to algs4.jar, and then sync to git). That is, git is just an externally facing way to report bugs and distribute the code, but the workflow does not allow me to accept any pull requests directly. I didn't realize that would cause anyone to feel that they weren't being properly credited. But I certainly credit you here!

@Horki
Copy link
Author

Horki commented Aug 3, 2020

@kevin-wayne commit in algs4 repo would mean something, however, kudos here is pretty invisible and therefore useless.
It would be nice to have a notice about the pull requests policy in README file, so there won't be any confusion.

@DyslexicAtheist
Copy link

Right, just confirming that it has no positive effects on correctness either. It's a stylistic improvement and fixes a loitering (memory) bug.

how can it be a "stylistic improvement" when in the same sentence it's also a "memory bug". Sounds like downplaying tbh.

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.

None yet

3 participants