Harmony
  1. Harmony
  2. HARMONY-403

HashMap hashcode ignores values in entries

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Classlib
    • Labels:
      None
    • Estimated Complexity:
      Moderate

      Description

      While it is obviously trivial to create different HashMaps with identical hashCodes, I'd still expect the following test code to pass:

      HashMap map1 = new HashMap(10);
      HashMap map2 = new HashMap(10);
      map1.put("key", "1");
      map2.put("key", "2");
      assertFalse(map1.hashCode() == map2.hashCode());

      That is, I'd expect the 'value' associated with "key" to have some impact on the hashCode. It passes on the reference implementations I tested.

      1. fix.hashmap.hashcode.diff
        0.7 kB
        Mark Hindess
      2. hashmap.hashcodes.should.differ.diff
        1 kB
        Mark Hindess

        Issue Links

          Activity

          Mark Hindess created issue -
          Hide
          Mark Hindess added a comment -

          Attaching a test case.

          Show
          Mark Hindess added a comment - Attaching a test case.
          Mark Hindess made changes -
          Field Original Value New Value
          Attachment fix.hashmap.hashcode.diff [ 12325764 ]
          Hide
          Mark Hindess added a comment -

          (Oops, attached these in the wrong order but JIRA wont really show this so I should have kept quiet.)

          The file "fix.hashmap.hashcode.diff" is a fix that I thought might fix the problem but actually doesn't. However, it still "smells" like a bug to me.

          Show
          Mark Hindess added a comment - (Oops, attached these in the wrong order but JIRA wont really show this so I should have kept quiet.) The file "fix.hashmap.hashcode.diff" is a fix that I thought might fix the problem but actually doesn't. However, it still "smells" like a bug to me.
          Mark Hindess made changes -
          Attachment hashmap.hashcodes.should.differ.diff [ 12325766 ]
          Hide
          Nathan Beyer added a comment -

          According to the Object.hashCode() specification this is not a guaranteed property [1].

          1. It is not required that if two objects are unequal according to the equals(java.lang.Object) method, then calling the hashCode method on each of the two objects must produce distinct integer results. However, the programmer should be aware that producing distinct integer results for unequal objects may improve the performance of hashtables.

          Is there some behavior you've found that requires this to be true?

          Show
          Nathan Beyer added a comment - According to the Object.hashCode() specification this is not a guaranteed property [1] . It is not required that if two objects are unequal according to the equals(java.lang.Object) method, then calling the hashCode method on each of the two objects must produce distinct integer results. However, the programmer should be aware that producing distinct integer results for unequal objects may improve the performance of hashtables. Is there some behavior you've found that requires this to be true?
          Hide
          Mark Hindess added a comment -

          Nathan, that test case was a very bad example. This test is better:

          public void test_hashCode()

          { HashMap map = new HashMap(10); Integer key = new Integer(1); Integer val = new Integer(2); map.put(key, val); int expected = key.hashCode() ^ val.hashCode(); assertEquals(expected, map.hashCode()); key = new Integer(4); val = new Integer(8); map.put(key, val); expected += key.hashCode() ^ val.hashCode(); assertEquals(expected, map.hashCode()); }

          Since the hashCode for Integer is defined as:

          a hash code value for this object, equal to the primitive int value
          represented by this Integer object.

          and the hashCode for HashMap (actually from AbstractMap) is:

          This implementation iterates over entrySet(), calling hashCode on
          each element (entry) in the Collection, and adding up the results.

          then the hashCode value in the above tests are well-defined for any RI. This tests fails on Harmony.

          Show
          Mark Hindess added a comment - Nathan, that test case was a very bad example. This test is better: public void test_hashCode() { HashMap map = new HashMap(10); Integer key = new Integer(1); Integer val = new Integer(2); map.put(key, val); int expected = key.hashCode() ^ val.hashCode(); assertEquals(expected, map.hashCode()); key = new Integer(4); val = new Integer(8); map.put(key, val); expected += key.hashCode() ^ val.hashCode(); assertEquals(expected, map.hashCode()); } Since the hashCode for Integer is defined as: a hash code value for this object, equal to the primitive int value represented by this Integer object. and the hashCode for HashMap (actually from AbstractMap) is: This implementation iterates over entrySet(), calling hashCode on each element (entry) in the Collection, and adding up the results. then the hashCode value in the above tests are well-defined for any RI. This tests fails on Harmony.
          Nathan Beyer made changes -
          Assignee Nathan Beyer [ nbeyer ]
          Nathan Beyer made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Hide
          Nathan Beyer added a comment -

          I've applied the patch at r418844, plus an additional fix. The patch hashCode calculation didn't completely fix the problem; the test case still failed. The problem seemed to be that the 'put' didn't instantiate an Entry correctly on the new key case; it wasn't passing the value to the Entry constructor, so the value was always missed.

          Let me know if this resolves the issue properly. Thanks.

          Show
          Nathan Beyer added a comment - I've applied the patch at r418844, plus an additional fix. The patch hashCode calculation didn't completely fix the problem; the test case still failed. The problem seemed to be that the 'put' didn't instantiate an Entry correctly on the new key case; it wasn't passing the value to the Entry constructor, so the value was always missed. Let me know if this resolves the issue properly. Thanks.
          Nathan Beyer made changes -
          Status In Progress [ 3 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Hide
          Nathan Beyer added a comment -

          I'm reopening this, as my fix broke everything. I think I've narrowed it down to the real problem: invalid assumption about Map.Entry.hashCode's implementation.

          Show
          Nathan Beyer added a comment - I'm reopening this, as my fix broke everything. I think I've narrowed it down to the real problem: invalid assumption about Map.Entry.hashCode's implementation.
          Nathan Beyer made changes -
          Status Resolved [ 5 ] Reopened [ 4 ]
          Estimated Complexity Moderate
          Resolution Fixed [ 1 ]
          Hide
          Nathan Beyer added a comment -

          I believe this issue was introduced because of the changes made to fix HARMONY-206. The patch for this introduced a hashCode algorithm that was only based upon the key, which is invalid according to the Map.Entry specification [1].

          [1] http://java.sun.com/j2se/1.5.0/docs/api/java/util/Map.Entry.html#hashCode()

          Show
          Nathan Beyer added a comment - I believe this issue was introduced because of the changes made to fix HARMONY-206 . The patch for this introduced a hashCode algorithm that was only based upon the key, which is invalid according to the Map.Entry specification [1] . [1] http://java.sun.com/j2se/1.5.0/docs/api/java/util/Map.Entry.html#hashCode( )
          Nathan Beyer made changes -
          Link This issue is related to HARMONY-206 [ HARMONY-206 ]
          Hide
          Nathan Beyer added a comment -

          I've applied another fix for this, which should now work. Please check that it fixes this issue. Thanks.

          Show
          Nathan Beyer added a comment - I've applied another fix for this, which should now work. Please check that it fixes this issue. Thanks.
          Nathan Beyer made changes -
          Resolution Fixed [ 1 ]
          Status Reopened [ 4 ] Resolved [ 5 ]
          Hide
          Nathan Beyer added a comment -

          No response, so we'll assume it's okay to close.

          Show
          Nathan Beyer added a comment - No response, so we'll assume it's okay to close.
          Nathan Beyer made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

            • Assignee:
              Nathan Beyer
              Reporter:
              Mark Hindess
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development