Incorrect Equals in all primitive HashMaps
In all primitive type hashMaps the equals method is implemented incorrectly. Incorrect care is taken of the no_entry_values. Fix is simple: The following if-statement needs to be replaced from:
if ((this_value != that_value) && (this_value != this_no_entry_value) && (that_value != that_no_entry_value)) { return false; }
TO:
if ((this_value != that_value) && ( (this_value != this_no_entry_value) || (that_value != that_no_entry_value)) ) { return false; }
Comments (14)
-
reporter -
reporter Comments from original bug:
Anonymous:
If the maps should allow for differences between "null" and "0" elements, then an additional check needs be be added to check the state of a key. Suppose that I take two maps, in the first, I add the pair (1,1) and in the second, the pair (2,1). Then, in the first map, I adjust the value (1, -1) and in the second (2, -1). The maps now both contain one key, referring to 0, however these are different keys.
Rob Eden
Similar issue to 3432402. Not sure if this is the right thing to do or not
Anonymous
Similar issue, yes, but as explained there, either way the current implementation is wrong and needs to be fixed. This holds for Lists as well. There also the no_entry_value is handled incorrectly (see 3439390).
Anonymous
After looking through the templates it seems that the Object_E_CustomHashMap. template also handles this incorrectly, leading to non-symmetric equality. If in the first map, a value is stored that equals the no_entry_value of the second map, the problem shows.
-
reporter -
Any news on this bug. I reported it a long time ago and even provided the fix above, but it seems the latest Gnu Trove libraries still contain this bug in all primitive maps.
-
reporter I'll bump the priority and try to get to it Very Soon (TM). Feel free to vote for bugs in the future when you're interested in them. It helps communicate how many people care about something.
A test case would also be helpful, though I think I can figure it out on this one.
-
reporter - changed milestone to Next Minor Release
- marked as major
-
reporter The above fix wasn't sufficient to cover the following case:
assertNotEquals( MIL( 1, 0 ), MIL( 0, 0 ) ); // One: {1=0} Two: {0=0}
To fix this case I've added
if ( !that.containsKey( key ) ) return false;
(a couple lines) before the "if" block.This is all tested by TPrimitivePrimitiveHashMapTest.testTIntLongMapEquals()
-
reporter Fix pushed in c2b7691
-
reporter - changed milestone to 3.0.5.b1
-
reporter - changed milestone to 3.0.5.m1
-
reporter - changed version to 3.0.4
-
reporter - changed status to resolved
-
Indeed, the additional line is needed in case you use adjustValue to get the contents onto the no_entry_value.
Thanks for the fix!
-
reporter - changed milestone to 3.1
- Log in to comment
Original SF bug