HashMap and Hashcode

Question

Ideally, as per the definition, HashMap does not allow duplicate keys; but I couldn't understand the following logic.

I have defined an Employee class and overridden the equals and hashcode methods.

public class Employee {

    private String empID;
    public int hashCode() {
        int result = 17;
        result = 37 * result + empID.hashCode();
        return result;
    }
    public boolean equals(Object object) {
        boolean result = false;
        if (object instanceof Employee) {
            Employee employee = (Employee) object;
            result = (this.empID.equalsIgnoreCase(employee.getEmpID()));
        }
        return result;
    }
}
Employee e1 = new Employee("1");
Employee e2 = new Employee("2");

Map<Employee, String> empMap = new HashMap<Employee, String>();
empMap.put(e1, "Emp1");
empMap.put(e2, "Emp2");
Set<Employee> keys = empMap.keySet();
for (Employee e: keys)
{
    e.setEmpID("1");
}
for (Employee e: keys)
{
    System.out.println(e.getEmpID());
    System.out.println(e.hashCode());
}
System.out.println(empMap.get(new Employee("1")) );

I am always getting the value Emp1. How to get the value Emp2?

Answer 1

I believe you have your keys and values backwards. You should be using your String as the key and your Employee class as the value:

Map<String,Employee> empMap = new HashMap<>();
empMap.put("Emp1", e1);
empMap.put("Emp2", e2);

Then your Employee class doesn't need hashCode and equals methods at all. Your Employee class is not suitable for use as a map key, mainly because keys are not supposed to change their internal variables. The reason you only see one ID while iterating is because of this:

for (Employee e: keys)
{
    e.setEmpID("1");
}

That is broken for three reasons. (1) You mustn't modify a key's variables while it is in the map, because the map won't know about it. To change the key associated with a value, you must remove() that mapping from the map, and put() it back in with the new key. (2) Modifying the map wouldn't be allowed anyway if you're simultaneously iterating over the map (unless you do it through the map's iterator). (3) You're giving all employees the same key, so the map can't tell them apart.

Think about the purpose of your Employee class. If you really want to use it as a map key, remove the setter for the ID, since it will always be wrong to use that method. An ideal map key is completely immutable and final. It's also wrong to use equalsIgnoreCase in the equals method, because then the equals method is not consistent with the hashCode method. (If a.equals(b) is true then it is required that a.hashCode() == b.hashCode(), or the map will not work properly.) By the time you have finished fixing your Employee class for use as a map key, it is just an annoying and unnecessary wrapper around String, but then you have no place to put any other employee information, which is why I suggest simply swapping the keys and values, which will solve all of your problems in one go.

Answer 2

HashMap isn't working correctly because you modify values that affect the results of equals and hashCode. Ya can't do that. These methods have to be consistent. Generally, any fields that you use to implement equality should be final to avoid this mistake.

---

Also - this isn't an answer to the question, but you should read the contracts of equals and hashCode. Your Employee class violates their contracts.

If two objects are equal according to the equals(Object) method, then calling the hashCode method on each of the two objects must produce the same integer result.

Counterexample: Employee("a") and Employee("A").

---

Recommended reading: What issues / pitfalls must be considered when overriding equals and hashCode?