Incorrect Equals in all primitive HashMaps

Issue #2 resolved
Rob Eden created an issue

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;


if ((this_value != that_value) 
    && ( (this_value != this_no_entry_value) 
    || (that_value != that_no_entry_value))
    ) {
    return false;

Comments (14)

  1. Rob Eden reporter

    Comments from original bug:


    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


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


    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.

  2. 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.

  3. Rob Eden 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.

  4. Rob Eden 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()

  5. 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!

  6. Log in to comment