override hashCode() and equals() to store an object inside hashMap not working properly

Question

i overrided hashCode() and equals() in a class (Dog) in order to store and retrieve it's instances from a hashMap, the code is as follows:

class Dog {

public Dog(String n) {
    name = n;
}
public String name;

public boolean equals(Object o) {
    if ((o instanceof Dog)
            && (((Dog) o).name == name)) {
       return true;
    } else {
       return false;
    }
}

public int hashCode() {
    return name.length();
   }
}

and the hashMap code is as follows:

public class MapTest {

public static void main(String[] args) {
    Map<Object, Object> m = new HashMap<Object, Object>();
    m.put("k1", new Dog("aiko"));
    Dog d1 = new Dog("clover");
    m.put(d1, "Dog key");    // #1
    System.out.println(m.get("k1"));
    String k2 = "k2";
    d1.name = "arthur";     // #2
    System.out.println(m.get(d1)); #3 
    System.out.println(m.size()); 
  }
}

the problem is that, at 2 i changed the name of the dog object that's stored inside the hashMap at 1, the expected output at 3 is NULL but the actual is Dog Key!! i expect it to fail in the equals() method as clover!=arthur but it succeds!! i noticed that when the hashCode succeds (i.e. the lengh==6) the value stored in the map is retrieved even though the equals() method fails, i changed == and used equals() instead but no changes happens, the problem remains.

I really wish the Java compiler would warn/error on `x == y`, where the types of x/y are not primitive. (It does warn in C#/VS for various forms, BTW.) That is, I *really wish* an explicit `(object)x == (object)y` form (or other) was required .. this question/issue comes up *a good bit*. — , Aug 01 '12 at 22:53
Not entirely following what you are doing. If you modify the object in the map but keep the key constant, then you'll still be able to get the value using the same key instance. `dog.equals(dog)` will always be true with your code (unless there is concurrent modification). — Tom Hawtin - tackline, Aug 01 '12 at 22:59
@TomHawtin-tackline Please post as an answer .. I missed that, as did two other posters. :) — , Aug 01 '12 at 23:03

score 5 · Answer 1 · answered Aug 01 '12 at 22:48

5

You want to compare the strings using .equals() not ==, which compares references.

public boolean equals(Object o) {
    if ((o instanceof Dog)
            && (((Dog) o).name.equals(name))) {
       return true;
    } else {
       return false;
    }
}

Also that equals method is a little off. What if name is null? You'll get a null pointer exception. You need to add another check for that special case.

answered Aug 01 '12 at 22:48

Oleksi

12,947
4
56
80

Nothing changes it still returns Dog key – Eslam Hamdy Aug 01 '12 at 22:55
1

This doesn't appear to be relevant to the question - for a start, all the strings are interned. – Tom Hawtin - tackline Aug 01 '12 at 23:00
@Tom, how do you know all strings are interned? – Steve Kuo Aug 01 '12 at 23:13
@SteveKuo They all appear as literals in the source. I'm not sure that's a guarantee, but it's a pretty safe bet that they are interned. – Daniel Fischer Aug 01 '12 at 23:15
1

That's a bad assumption, as his source is just sample code. The moment the string value is read in from a database then `==` is broken. `equals` is THE correct way to compare strings. – Steve Kuo Aug 01 '12 at 23:26
1

@SteveKuo String literals are guaranteed by the language specification to be interned. Yes, you could change the source and it could do something different, but the question is about the behaviour of this exact source. – Tom Hawtin - tackline Aug 02 '12 at 03:35
@Tom your are incorrect. String are not guaranteed to be interned. Why do you think there's an explicit inter method? – Steve Kuo Aug 04 '12 at 17:12
1

@SteveKuo String literals are. Shouldn't be too hard to find in the JLS. – Tom Hawtin - tackline Aug 04 '12 at 18:29

score 3 · Accepted Answer · 2012-08-02T03:46:34.413

Why does `equals` "never fail"?

As per Tom's comment:

.. If you modify the object in the map but keep the key constant, then you'll still be able to get the value using the same key instance. dog.equals(dog) will always be true with your code (unless there is concurrent modification)

That is, the line:

d1.name = "arthur";

Is mutating the object already in the HashMap. Compare with (where t "prints" true or false):

Dog d1 = new Dog("clover");
// what "put" is effectively doing: there is *no* Copy/Clone
Dog inMap = d1;
t(inMap == d1);                 // true: same object, reference equality!
d1.name = "arthur";
t(inMap.name.equals("arthur")); // true: same object! "name" member was *mutated*
t(d1.equals(inMap));            // true: same object!

So the equals never fails because it is comparing the object with itself :)

I missed it at first too: remember Java has Call By Object-Sharing semantics. That is, there is no implicit Copy/Clone/Duplicate for objects that are passed to methods.

So then, how to get it to fail:

Dog d1 = new Dog("clover");
Dog d2 = new Dog("clover");
t(d1 == d2);                   // false: different objects!
m.put(d1, "Dog key");          // put in with D1 object
System.out.println(m.get(d1)); // "Dog key"   -okay, equals
System.out.println(m.get(d2)); // "Dog key"   -okay, equals
d2.name = "arthur";            // *mutate* D2 object
t(d1.equals(d2));              // false: no longer equal
System.out.println(m.get(d1)); // "Dog key"   -okay, always equals, as per above
System.out.println(m.get(d2)); // ""          -no good, no longer equals

And how does the `hashCode` fit in?

The hash code is used to determine the hash table bucket to put the key (and value pair) in. When performing a look-up (or set) the bucket is first looked up by hash code and then each key already mapped to the bucket is checked with equals. If there are no keys in the bucket then equals is never invoked.

This explains why changing the name to a String of length 8 in the original post results in a failing look-up: a different bucket (say, one that is empty) is initially chosen and thus equals is never invoked upon existing keys which exist in other buckets. The same object key might be already there, but it's never looked at!

So then, how to get it to fail with different hash code:

Dog d1 = new Dog("clover");
m.put(d1, "Dog key");          // put in with D1 object, hashCode = 6
System.out.println(m.get(d1)); // "Dog key" -okay, hashCode = 6, equals
d1.name = "Magnolia";          // change value such that it changes hash code
System.out.println(m.get(d1)); // ""        -fail, hashCode = 8, equals
// ^-- attaching a debugger will show d1.equals is not called

Thus, for a key to be found in a hash table (such as a HashMap) it must be such that:

k.hashCode() == inMap.hashCode() && k.equals(inMap);

There may be many hash codes that map to the same bucket. However the above is the only guarantee that a look-up will be successful.

Of course, see the other replies for correct way to compare strings in general.

if your answer is right, then why it returns NULL when i pass a name>6 chars (i.e. d1.name="Magnolia")? — Eslam Hamdy, Aug 01 '12 at 23:28
@Eslam Per not finding the correct bucket, as per the post. `hashCode()` is used to find the bucket *first*, and equals is only used on *objects found in the bucket*. (This is why `hashCode` should be *stable*.) — , Aug 01 '12 at 23:29
@Eslam Each `HashMap` entry contains a copy of the hash value when placed into the map. Even if there is a hash collision, the full 32-bit value of the hash is compared before attempting to look at key equality. — Tom Hawtin - tackline, Aug 02 '12 at 03:39

score 1 · Answer 3 · answered Aug 01 '12 at 23:00

1

Apart from the problem of comparing strings with ==, what is happening is that you change the dog's name

Dog d1 = new Dog("clover");
m.put(d1, "Dog key");    // #1
System.out.println(m.get("k1"));
String k2 = "k2";
d1.name = "arthur";     // #2

to one with the same length. Hence the lookup in the map looks in the same hashbucket, and of course there it finds the dog.

If you change the name to one with a different length, it will (generally) look in a different bucket and then not find the dog.

answered Aug 01 '12 at 23:00

Daniel Fischer

181,706
17
308
431

This is not true. Well, it is. But it is not complete. The OP explains the length and the bucket matching. This still doesn't explain the "equals not failing". – Aug 01 '12 at 23:02
sure it will look at the same hashBucket and this is the first step, then the equals() method comes in the second step to determine if the two are equal, and in fact they aren't – Eslam Hamdy Aug 01 '12 at 23:04
@Eslam But they *are* equal (which is why `equals` always returns true), see Tom's comment in the main post .. – Aug 01 '12 at 23:04
@Eslam `d1` is the key in the map, and the object you look up is _the same_ object. – Daniel Fischer Aug 01 '12 at 23:05

override hashCode() and equals() to store an object inside hashMap not working properly

3 Answers3

Why does equals "never fail"?

And how does the hashCode fit in?

Why does `equals` "never fail"?

And how does the `hashCode` fit in?