Incorrect Equals in all primitive HashMaps

[Rob Eden]

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;
}

[Rob Eden]

Original SF bug

[Rob Eden]

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.

[Boudewijn van Dongen]

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.

[Rob Eden]

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.

[Rob Eden]

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()

[Rob Eden]

Fix pushed in c2b7691

[Boudewijn van Dongen]

Indeed, the additional line is needed in case you use adjustValue to get the contents onto the no_entry_value.

Thanks for the fix!