CODEWITHSUNDEEP
javaoverridingequalshashcode
WilliamShatner
WilliamShatner

Reputation: 926

Did I override equals and hashcode correctly?

In my most recent question, I was informed that I needed to override my equals and hashcode method (among other things). So I took a bit of time to read a few articles and attempted to come up with a proper implementation.

Here are some of the articles I read:

All the articles were pretty good. Since this is my first time attempting to do this, I just want to be sure I'm not making some simple (or dumb) mistake.

I will be using name to indicate whether my Person object is equivalent to another Person object. The reason for this, is that all the other variables can vary, but the name will always be unique.

Updated to reflect recommended changes

public class Person {

    private String name;
    private int p_number;
    private String address;
    //other variables

    public Person(String a_name) {
        name = a_name;
    }

    public String getName() {
        return name;
    }

    //other getters and setters

    @Override
    public boolean equals(Object o) {
        if(o == null) 
            return false;

        if(o == this) 
            return true;

        if(!(o instanceof Person)) 
            return false;

        Person p = (Person) o;
        return name.equals(p.name));

    }

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

My questions are as follows:

  1. Did I implement these methods correctly?
  2. Since name is the only variable that determines uniqueness, do I need to bother checking any of the other variables in hashcode?
  3. I was reading on StackOverflow that 31 was chosen as a good prime number awhile ago, but choosing a larger prime is better now? Can someone confirm or deny this claim? (the claim was made in the third link above)

If I haven't implemented these methods properly, how can I change/improve them?

Upvotes: 0

Views: 277

Answers (2)

fge
fge

Reputation: 121712

In equals():

if(name.equals(p.getName()))
    return true;

Missing false, and you can just:

// Both are Person instances, no need to use the accessor here
return name.equals(p.name);

As to hashCode(), just return name.hashCode();

Also, can name be null? Your methods don't seem to account for that. (edit: answer: no)

As to your questions:

Since name is the only variable that determines uniqueness, do I need to bother checking any of the other variables in hashcode?

NO, certainly not! If your names are equal but ages different, this would lead to a different hash code for objects which are equal, and this violates the Object contract!

I was reading on StackOverflow that 31 was chosen as a good prime number awhile ago, but choosing a larger prime is better now? Can someone confirm or deny this claim? (the claim was made in the third link above)

This, no idea...

To be more complete about the .equals()/.hashCode() contract, I'll mention a utility class from Guava: Equivalence. An implementation of this abstract class for a given class can allow you to create Sets, and therefore Maps, with these objects as members (keys) as if they had a different implementation of both of these functions:

Equivalence<MyClass> eq = ....;

Set<Equivalence.Wrapper<MyClass>> set = ...;

set.add(eq.wrap(myClassInstance));

This can be actually very useful in some scenarios...

Upvotes: 3

greedybuddha
greedybuddha

Reputation: 7507

Your equals needs to return a value in all cases, just change the end part to return the name.equals.

@Override
public boolean equals(Object o) {
    ...
    return name.equals(o.getName());
}

Also your hashcode is actually detrimental, all it does is use the name.hashCode() but then multiply it by 31, better to use the default Java string hashcode from the name directly.

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

Upvotes: 1

Related Questions