tags:

views:

2749

answers:

8

Hi, I am trying to locate a key in a HashMap. I can print the selected key by using 'get' but when I use 'containsKey' in an if statement, it is not found.

I KNOW the key is present in the Map but it keeps returning false. Any ideas people?

My code:

public static boolean checkLowerStructuralSupport(Location location) {

boolean hasSupport = false;

Location supportingLocation = new Location(location.getX(), location.getY(), location.getZ() -1);

System.out.println( _levels.get(supportingLocation.getZ()).getLevelSites2().get(supportingLocation)); //works

     if (_levels.get(supportingLocation.getZ()).getLevelSites2().containsKey(supportingLocation)) {
      hasSupport = true;
     }
     else {
      hasSupport = false;
     }

     return hasSupport;
}

Here is the code for the Location class:

public class Location {

    protected int _x;
    protected int _y;
    protected int _z;

    public Location(int xAxis, int yAxis, int zAxis) {
     this._x = xAxis;
     this._y = yAxis;
     this._z = zAxis;
    }

    public void equals() {
     //not implemented yet
    }

    public void HashCode() {
     //not implemented yet
    }

    public String toString() {
     String locationString = Integer.toString(_x) + Integer.toString(_y) + Integer.toString(_z);
     return locationString;
    }

    public void setX(int XAxis) {
     this._x = XAxis;
    }

    public int getX() {
     return this._x;
    }

    public void setY(int YAxis) {
     this._y = YAxis;
    }

    public int getY() {
     return this._y;
    }

    public void setZ(int ZAxis) {
     this._z = ZAxis;
    }

    public int getZ() {
     return this._z;
    }

}
+9  A: 

You must ensure that the Location class has properly implemented its hashCode() and equals(Object) methods (documentation). That is, if two Location objects are effectively equal, they should share a common hash code and their equals method should return true.

Adam Paynter
But then why would get() work and containsKey() not?
butterchicken
@butterchicken: Maybe get() is not using "hashCode" optimization to get the value, while "containsKey" is.
Pablo Santa Cruz
I have used containsKey effectively in other methods....frazzled
burntsugar
Is there any more to the Location class than you are showing? As shown, equals and hashCode have the wrong signatures (and spelling), so if Location inherits from a superclass, then maybe that is defining a broken implementation of either.get/containsKey (in JDK1.6 at least) look to have equivalent source to me, so I think that something must be affecting your data in the meantime.
paulcm
+2  A: 

In Location class, make sure you are overriding hashCode and equals methods.

If you are, can you post them?

Pablo Santa Cruz
+1  A: 

Both get() and containsKey() are using the Location class's hashCode() method. The equals() method isn't called unless there is a hash collision. (thus, HashMap's get() won't use equals() in every situation.)

For your Location class, did you by chance happen to implement your own version of hashCode()? The hashCode() method should be implemented carefully. Joshua Bloch described all the details in the book Effective Java, portions of which are online... I'll go find the link to those sample chapters: Effective Java Sample Chapters. You want chapter 3.

As I asked in the comment to the question, Where does your _levels variable come from? I don't see it declared inside that method and your naming (underscore prefix, are you importing that convention from some other language?) suggests that it "lives" outside this method. Perhaps other code is changing it during execution? Please let us know when you solve it; the suspense is killing me.

dustmachine
I'd think they both use equals(). Keep in mind, it's perfectly legal for two instances of an object to return the `hashCode()`. In fact, it isn't even illegal for all `hashCode()` to simply return the same constant (e.g., 0), just really really sub-optimal.
Jack Leow
paulcm
equals will get called during a get if there's a hash collision.
Steve Kuo
+2  A: 

containsKey uses the method equals to compare the param with the entries in the key set. So the Location class needs to have a equals method that is good. The default equals method in java.lang.Object only returns true when both objects are the same object. In this case you probably have 2 different instances that needs to be compared and need a custom equals method.

+1  A: 

To avoid problems, your equals() and hashCode() methods should be consistent and conform to the requirements (as noted elsewhere).

Additionally, hashCode() should not rely on mutable members, otherwise your calculated hash code can change, and this affects the internal workings of the HashMap. That will reveal itself in an inability to retrieve stuff from Hash* collections.

Brian Agnew
+5  A: 

As descibed here, you have to override the equals(Object) method.

The reason why get(Object) is working is, that HashMap will calculate the Hash for your Location class and returns the Object the hascode points to.

containsKey(Object) calculates the hash key and gets the object the hash is pointed to. The object from the HashMap will compare to the Object you put in. For these comparison the equals method is used. When you do not override he equals method, true is returned, when the object reference to the same instance.

From HashMap

/** 
 * Check for equality of non-null reference x and possibly-null y. 
 */
static boolean eq(Object x, Object y) {
    return x == y || x.equals(y);
}

From Object

public boolean equals(Object obj) {
    return (this == obj);
    }

From the javadoc of equals

The equals method for class Object implements the most discriminating possible equivalence relation on objects; that is, for any non-null reference values x and y, this method returns true if and only if x and y refer to the same object (x == y has the value true).

Note that it is generally necessary to override the hashCode method whenever this method is overridden, so as to maintain the general contract for the hashCode method, which states that equal objects must have equal hash codes.

Markus Lausberg
I could be wrong, but I'm pretty sure get(...) will compare the key (Location) for equality also, and uses the hash only to locate it quickly (reduce the number of comparisons). How else would it distinguish between multiple objects with the same hash (which is perfectly legal).
Jack Leow
+1  A: 

The only thing I can think of that will cause this is if the state of supportingLocation is somehow being mutated between the get(...) call and the containsKey(...).

Assuming the code snippet you posted is the exact code that's causing problems, the only place this could occur is if one of Location#getZ(...), Location#hashCode() or Location#equals(Object) mutates the state of Location (or the Location constructor, or one of these methods starts a thread that randomly changes the state of the Location instance, but I think we can rule that out).

Could you verify that none of the methods above are changing the state of the supportingLocation instance? While I am not familiar with the Location class itself, I'd venture to guess that a class like that would ideally be immutable.

Edit: To clarify, when I say that Location#getZ() etc aren't mutating the Location, what I mean is:

Location x = new Location(1,2,3);
Location y = new Location(1,2,3);

boolean eq1 = x.equals(y);
int hash1 = x.hashCode();
x.getZ(); // this should *not* mutate the state of x
boolean eq2 = x.equals(y);
int hash2 = x.hashCode();

In the end, eq1 should be equal to eq1, and hash1 should be equal to hash2. If this is not the case, getZ() is mutating the state of x (or equals, or hashCode, or worse, those methods are completely off), and will result in the behavior you observed.

Jack Leow
The code I have posted is the exact implementation. The Location class has mutator methods. Cheers.
burntsugar
Mutation should not matter. equals and hashCode have not been overridden, so the default versions will be used, and they only look at the pointer, not the state of the object.
finnw
Yup, I agree, my answer was posted before the full Location source code was posted. I'm at a loss to what's wrong now, perhaps a rogue `Map` implementation, or a bug in `getLevelSites2()`.
Jack Leow
+1  A: 

Take a peak at the source code for the HashMap implementation. Both get and containsKey use the hasCode() and equals() methods of your key object.

The only real difference, and as was pointed out, it is a trivial null check, is in the comparisons:

get:

((k = e.key) == key || key.equals(k))

containsKey:

((k = e.key) == key || (key != null && key.equals(k)))

where e is of type Entry for a HashMap.

So, if you do not have a strong implementations of hashCode() and/or equals() you will have a problem. Additionally, if your keys were mutated (I see that you did not declare the class fields final) you could have an issue.

Take the following example:

public class HashMapTest {
    static class KeyCheck {
        int value;
        public KeyCheck(int value) { this.value = value; }
        public void setValue(int value) { this.value = value; }
        @Override public int hashCode() { return value; }
        @Override public boolean equals(Object o) {
            return ((KeyCheck)o).value == this.value;
        }
    }

    public static void main(String args[]) {
        HashMap<KeyCheck, String> map = new HashMap<KeyCheck, String>();
        KeyCheck k1 = new KeyCheck(5);
        KeyCheck k2 = new KeyCheck(5);

        map.put(k1, "Success");

        System.out.println("Key: " + k1 + " Get: " + map.get(k1) +
                           " Contains: " + map.containsKey(k1));
        System.out.println("Key: " + k2 + " Get: " + map.get(k2) +
                           " Contains: " + map.containsKey(k2));

        k1.setValue(10);

        System.out.println("Key: " + k1 + " Get: " + map.get(k1) +
                           " Contains: " + map.containsKey(k1));
        System.out.println("Key: " + k2 + " Get: " + map.get(k2) +
                           " Contains: " + map.containsKey(k2));
    }
}

This will print out:

Key: HashMapTest$KeyCheck@5 Get: Success Contains: true

Key: HashMapTest$KeyCheck@5 Get: Success Contains: true

Key: HashMapTest$KeyCheck@a Get: null Contains: false

Key: HashMapTest$KeyCheck@5 Get: null Contains: false

As you can see, in this case the mutability caused the hashCode() to change, which ruined everything.

Aaron K
I don't think that the difference between get and containsKey is significant, because it is only for when key is null, and in that case get has a special case code path (which will already have returned). Your point about mutating key values is well made though.
paulcm