The dreaded equals() and hashcode()

Today I finally got to the bottom of a bug that has been causing a headache. The solution was too obvious and text book that I thought I would publicly humiliate myself so as help me remember never to let it happen again.

The story is this:
I created a class called Landmark:

public class Landmark {
    /**
     * Key uniquely identifying landmark.
     */
    private long id;
    /**
     * Textual representation of the landmark.
     */
    private String name;
 
    /**
     * Constructor creates a landmark with id and name.
     * @param id
     * @param name
     */
    public Landmark(long id, String name) {
        this.id = id;
        this.name = name;
    }
 
    /**
     * Creates a landmark object with only an id.
     * @param id
     */
    public Landmark(long id) {
        this.id = id;
    }
....
....

All looks pretty reasonable, however notice the overloaded constructor, you can make a landmark just with the id, which happens to be really handy. Now see if you can spot the problem!

/**
     * Equality is only checked on id
     * @param obj
     * @return
     */
    @Override
    public boolean equals(Object obj) {
        if (obj == null) {
            return false;
        }
        if (getClass() != obj.getClass()) {
            return false;
        }
        final Landmark other = (Landmark) obj;
        if (this.id != other.id) {
            return false;
        }
        /*
        if ((this.name == null) ? (other.name != null) : !this.name.equals(other.name)) {
            return false;
        }*/
        return true;
    }
 
    @Override
    public int hashCode() {
        int hash = 3;
        hash = 53 * hash + (int) (this.id ^ (this.id >>> 32));
        hash = 53 * hash + (this.name != null ? this.name.hashCode() : 0);
        return hash;
    }

Ok so notice that equality was originally checked on name and id, thus is the name was missing they were not the same. You can imagine this became a real pain in the arse, because they should really be the same because they refer to the same landmark. So you can see one day in a fit of rage I simply commented out the line comparing the names and settled on equality based only on id. Spotted the issue yet?

Well being in a fit of rage I didn’t think it through very far, or look a few lines below, at the hashcode implementation. So now equality is defined as simply equality of id but hashcode is calculated from id and name! Guess what happened?

Well I was stashing some Landmarks in a hashmap as a key with some Integer values associated with them. I had a second Set of Landmarks which I needed the Integer for, so I happily try and pull out the Integer using one of my Landmarks. However the Landmark in the hashmap has a title, the Landmark I am using to pull the int doesn’t. Result? can’t retrieve a single value and get null pointer exceptions all over the shot! Why? Because the two landmarks I consider the same and that the .equals(object) method consider the same aren’t gonna be in the same place in the hashtable, so as far as the hash table is concerned, they sure aint the same!

Result? Give thought to equals() and hashcode() and remember what they do. All in all I learnt a good lesson at the expense of some real pain even if it was a programming 101 lesson