Fixed MIDIObject's hash implementation #181
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
armadsen
reviewed
Feb 20, 2017
Source/MIKMIDIObject.m
Outdated
| - (void)setUniqueID:(MIDIUniqueID)uniqueID | ||
| { | ||
| _uniqueID = uniqueID; | ||
| _hash = @(uniqueID).hash; |
There was a problem hiding this comment.
Did profiling indicate that caching the hash value actually makes an important performance difference in a real application? If not, I'd probably prefer the simpler approach of just doing -(NSUInteger)hash { return [@(self.uniqueID) hash]; }
There was a problem hiding this comment.
I did not do any profiling, just went for the best performance approach, just adds a bunch of lines, so… as you wish feel free to change that ;)
Fixed MIDIObject's hash implementation
|
Thanks for this! Finally merging it now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
MIDIUniqueID's type being SInt32, ie. an int, it is not directly compatible with -hash which is unsigned int. I've seen many unique IDs being negative and when they get cast to UInt, their value ends up being18446744073709551160ie.NSUIntegerMaxwhich is not at all unique.Hence, we start to return similar hashes for different MIDI objects.
This simple fix uses NSNumber's native hash implementation.