Code Review · Java

Java Code Review: Bad HashMap Key Design

Review Java code that uses a mutable object as a HashMap key and causes lookups to fail after the key changes.

Code ReviewJavaHashMapequalshashCodeImmutability

Code Review Exercise

Review the following code. It compiles, and at first glance it may look reasonable. What problems do you see?

java

import java.util.HashMap;
import java.util.Map;

class UserKey {
    private String email;

    UserKey(String email) {
        this.email = email;
    }

    public String getEmail() {
        return email;
    }

    public void setEmail(String email) {
        this.email = email;
    }

    @Override
    public boolean equals(Object obj) {
        if (!(obj instanceof UserKey other)) {
            return false;
        }

        return email.equals(other.email);
    }

    @Override
    public int hashCode() {
        return email.hashCode();
    }
}

public class Demo {
    public static void main(String[] args) {
        Map<UserKey, String> sessions = new HashMap<>();

        UserKey key = new UserKey("alice@example.com");
        sessions.put(key, "SESSION-123");

        key.setEmail("new-alice@example.com");

        System.out.println(sessions.get(key)); // null
    }
}

Before reading the review, ask yourself: why does the lookup return null even though we are using the same object reference?

Review Findings

Issue #1: The HashMap key is mutable

UserKey is used as a key in a HashMap, but its email field can be changed after insertion. This is dangerous because email is also used by equals() and hashCode().

Issue #2: hashCode() changes after insertion

A HashMapuses the key's hash code to decide where to store the entry. If the hash code changes later, the map may look in the wrong bucket during lookup.

Issue #3: equals() and hashCode() depend on a nullable field

If email can ever be null, both equals()and hashCode() can throw NullPointerException. A reviewer should ask whether null is allowed and enforce that contract clearly.

Issue #4: The domain model is unclear

If email identifies a user, should it be mutable? If users can change email addresses, then email may not be the best long-term identity key. A stable user id may be safer.

Why This Happens

A HashMap does not scan every key from top to bottom. It uses hashCode() to quickly decide where a key should live internally.

When this line runs, the map stores the entry based on the hash code of alice@example.com:

java

sessions.put(key, "SESSION-123");

Then this line changes the value used by hashCode():

java

key.setEmail("new-alice@example.com");

Now the same object produces a different hash code. The map looks in a different place and cannot find the original entry.

Better Version: Make the Key Immutable

If an object is going to be used as a map key, the fields used inequals() and hashCode() should usually be immutable.

java

import java.util.Objects;

final class UserKey {
    private final String email;

    UserKey(String email) {
        this.email = Objects.requireNonNull(email, "email must not be null");
    }

    public String getEmail() {
        return email;
    }

    @Override
    public boolean equals(Object obj) {
        if (this == obj) return true;
        if (!(obj instanceof UserKey other)) return false;
        return email.equals(other.email);
    }

    @Override
    public int hashCode() {
        return email.hashCode();
    }
}

This version removes the setter, makes the field final, and rejects null values at construction time.

Often Better: Use a Stable Identifier

In many real systems, a user can change their email address. If so, email may be a bad identity key. A stable id is usually better.

java

final class UserKey {
    private final long userId;

    UserKey(long userId) {
        this.userId = userId;
    }

    @Override
    public boolean equals(Object obj) {
        if (this == obj) return true;
        if (!(obj instanceof UserKey other)) return false;
        return userId == other.userId;
    }

    @Override
    public int hashCode() {
        return Long.hashCode(userId);
    }
}

This is not just a code-style improvement. It is a design decision: choose a key that represents stable identity.

What A Strong Interview Review Sounds Like

A strong reviewer would not only say, "this returns null." They would explain the underlying contract:

If an object is used as a HashMap key, any fields used by equals() and hashCode() should not change while the object is in the map. Otherwise the map may not be able to find the entry again.

Interview Follow-Ups

Is it always wrong to use custom objects as HashMap keys?

No. It is common and useful. The key object just needs a correct equals() and hashCode(), and the fields used by those methods should be stable while the object is in the map.

Does HashMap use equals() or hashCode()?

It uses both. hashCode() helps find the bucket. equals() is then used to compare keys within that bucket.

Can two unequal objects have the same hashCode()?

Yes. That is called a hash collision. HashMap handles collisions by checking equals() among keys in the same bucket.

What happens if equals() is overridden but hashCode() is not?

The object may behave incorrectly in hash-based collections such as HashMap and HashSet because equal objects may not produce the same hash code.

What is the safest design for a HashMap key?

Use an immutable object with final fields, a clear equality definition, and a stable identifier that will not change after insertion.

Final Takeaway

The review issue is not merely "HashMap returned null." The deeper issue is key design. If the key's identity can change after it is inserted into the map, the map can no longer reliably find it.