75

Is there a specific rule on how Overriding equals() & hashCode() in sub classes considering super fields ?? knowing that there is many parameters : super fields are private/public , with/without getter ...

For instance, Netbeans generated equals() & hashCode() will not consider the super fields ... and

    new HomoSapiens("M", "80", "1.80", "Cammeron", "VeryHot").equals(
    new HomoSapiens("F", "50", "1.50", "Cammeron", "VeryHot"))

will return true :(

public class Hominidae {

    public String  gender;
    public String  weight;
    public String  height;

    public Hominidae(String gender, String weight, String height) {
        this.gender = gender;
        this.weight = weight;
        this.height = height;
    }
    ... 
}

public class HomoSapiens extends Hominidae {
    public String name;
    public String faceBookNickname;

    public HomoSapiens(String gender, String weight, String height, 
                       String name, String facebookId) {
        super(gender, weight, height);
        this.name = name;
        this.faceBookNickname = facebookId;
    }
    ...  
}

If you want to see the Netbeans generated equals() & hashCode() :

public class Hominidae {

    ...

    @Override
    public boolean equals(Object obj) {
        if (obj == null) {
            return false;
        }
        if (getClass() != obj.getClass()) {
            return false;
        }
        final Hominidae other = (Hominidae) obj;
        if ((this.gender == null) ? (other.gender != null) : !this.gender.equals(other.gender)) {
            return false;
        }
        if ((this.weight == null) ? (other.weight != null) : !this.weight.equals(other.weight)) {
            return false;
        }
        if ((this.height == null) ? (other.height != null) : !this.height.equals(other.height)) {
            return false;
        }
        return true;
    }

    @Override
    public int hashCode() {
        int hash = 5;
        hash = 37 * hash + (this.gender != null ? this.gender.hashCode() : 0);
        hash = 37 * hash + (this.weight != null ? this.weight.hashCode() : 0);
        hash = 37 * hash + (this.height != null ? this.height.hashCode() : 0);
        return hash;
    }

}


public class HomoSapiens extends Hominidae {

    ...

    @Override
    public boolean equals(Object obj) {
        if (obj == null) {
            return false;
        }
        if (getClass() != obj.getClass()) {
            return false;
        }
        final HomoSapiens other = (HomoSapiens) obj;
        if ((this.name == null) ? (other.name != null) : !this.name.equals(other.name)) {
            return false;
        }
        if ((this.faceBookNickname == null) ? (other.faceBookNickname != null) : !this.faceBookNickname.equals(other.faceBookNickname)) {
            return false;
        }
        return true;
    }

    @Override
    public int hashCode() {
        int hash = 7;
        hash = 89 * hash + (this.name != null ? this.name.hashCode() : 0);
        hash = 89 * hash + (this.faceBookNickname != null ? this.faceBookNickname.hashCode() : 0);
        return hash;
    }
}
wj.
  • 2,001
  • 3
  • 18
  • 19

10 Answers10

73

Children should not examine the private members of their parents

But obviously, all significant fields should be taken into account for equality and hashing.

Fortunately, you you can easily satisfy both rules.

Assuming you're not stuck using the NetBeans-generated equals and hashcode, you can modify Hominidae's equals method to use instanceof comparison rather than class equality, and then use it straightforwardly. Something like this:


    @Override  
    public boolean equals(Object obj) {  
        if (obj == null) { return false; }  
        if (getClass() != obj.getClass()) { return false; }  
        if (! super.equals(obj)) return false;
        else {
           // compare subclass fields
        }

Of course, hashcode is easy:


    @Override     
    public int hashCode() {     
        int hash = super.hashCode();
        hash = 89 * hash + (this.name != null ? this.name.hashCode() : 0);     
        hash = 89 * hash + (this.faceBookNickname != null ? this.faceBookNickname.hashCode() : 0);     
        return hash;     
    }     

Seriously, though: what's up with NetBeans not taking superclass fields into account by calling the superclass methods?

CPerkins
  • 8,968
  • 3
  • 34
  • 47
  • @CPerkins : it's hard to automatize this generation, think about the case of private fields with/without getter ... I think that's why IDE's don't do it ... – wj. Jan 14 '10 at 21:27
  • @wj, all you have to do is call the superclass equals and hashcode methods, and the fields are taken into account. IDEs could easily do this. Children should not have to explicitly examine the private members of their parents. – CPerkins Jan 15 '10 at 14:57
  • @CPerkins: It's a little bit more complicated, in that the IDE has to know if one of the parent classes is overriding `equals()` and `hashCode()`. If they're not, then calling `super.hashCode()` or `.equals()` will use `Object`'s implementation, which would be incorrect to factor into the computation (as it is instance-based, not value-based). – Mark Peters May 23 '12 at 17:41
  • @MarkPeters Agreed in the general case. OP's question was about a case in which all clases in the inheritance tree do override equals and hashcode. But in the case where that's not true, you're already into a bad smell: fix your base classes before trying to use them in a context requiring a functional equals/hashcode. – CPerkins May 24 '12 at 14:17
  • 70
    I think "Children should not examine the private members of their parents" is very sound parental advice. – OliCoder Nov 26 '12 at 11:15
  • 21
    This `equals` method does not follow the [equals contract](http://docs.oracle.com/javase/7/docs/api/java/lang/Object.html#equals%28java.lang.Object%29). It is not symmetric, because `subclassInstance.equals(parentInstance)` can never return true due to the getClass() check, but implicit in your use of `super.equals` is the assumption that `parentInstance.equals(subclassInstance)` can return true. – MikeFHay May 14 '14 at 10:06
  • Shouldn't the class check be changed into `isAssignableFrom()`? – Adriaan Koster Jun 15 '16 at 13:23
  • 2
    If the `super#equals` works the same, `super.equals(obj)` always returns `false`. – Jin Kwon Aug 20 '16 at 16:49
  • Just FYI You can get Eclipse to generate the two methods for you: Source > Generate hashCode() and equals(). – Mahender Reddy Yasa Jul 28 '17 at 10:10
  • 1
    I opened a issue for NetBeans https://issues.apache.org/jira/browse/NETBEANS-4512 – Pavel_K Jun 28 '20 at 07:51
  • 1
    @MikeFHay `super.equals` is only called for instances of the same class, `this.getClass()` will still return the child class even if it's inside the parent's `equals` method. Symmetry isn't broken here. – algrid Jun 21 '21 at 16:43
  • @algrid super.equals is also called if an instance of the concrete superclass has `equals` called on it directly. – MikeFHay Jun 21 '21 at 17:04
23

I prefer to use EqualsBuilder (and HashcodeBuilder) from the commons-lang package to make my equals() and hashcode() methods a lot easier to read.

Example:

public boolean equals(Object obj) {
 if (obj == null) { return false; }
 if (obj == this) { return true; }
 if (obj.getClass() != getClass()) {
   return false;
 }
 MyClass rhs = (MyClass) obj;
 return new EqualsBuilder()
             .appendSuper(super.equals(obj))
             .append(field1, rhs.field1)
             .append(field2, rhs.field2)
             .append(field3, rhs.field3)
             .isEquals();
}
matt b
  • 138,234
  • 66
  • 282
  • 345
10

Generally speaking implementing equals across subclasses is hard to keep symmetric and transitive.

Consider a superclass that checks for field x and y, and subclass checks for x, y and z.

So a Subclass == Superclass == Subclass where z is different between the first instance of Subclass and the second, violating the transitive part of the contract.

This why the typical implementation of equals will check for getClass() != obj.getClass() instead of doing an instanceof. In the above example, if SubClass or Superclass does an instanceof check it would break symmetry.

So the upshot is that a subclass can certainly take into account super.equals() but should also do its own getClass() check to avoid the above issues and then check for equals on its own fields in addition. It would be a strange duck of a class that changed its own equals behavior based on specific fields of the superclass rather than just if the superclass returns equals.

Jin Kwon
  • 20,295
  • 14
  • 115
  • 184
Yishai
  • 90,445
  • 31
  • 189
  • 263
  • thx Yishai, but the issue here is to compare 2 instances of subclass and the problem is that we can not do something like "super.equals(obj.super)" where obj is the compared object – wj. Jan 14 '10 at 20:08
  • @wj, As long as your class and the obj class are the same, I don't get why you can't call `if (!super.equals(obj)) return false' – Yishai Jan 14 '10 at 20:37
  • @Yishai, the point here is to find a way to compare "this.super" and "obj.super" not "this.super" and "obj" because their not direct instance of the same class meaning that "super.equals(obj)" is always false ... in my example "this.super" is an "Hominidae" when "obj" is an "HomoSapiens" – wj. Jan 15 '10 at 04:53
  • @wj, I don't think you understand me. I'm suggesting the same thing as the appendSuper method in the Equals builder in @matt b's answer. – Yishai Jan 15 '10 at 14:28
  • @Yishai What is your suggestion amigo??? – Jack Mar 02 '23 at 18:28
7

The rules are:

  • It is reflexive: for any non-null reference value x, x.equals(x) should return true.
  • It is symmetric: for any non-null reference values x and y, x.equals(y) should return true if and only if y.equals(x) returns true.
  • It is transitive: for any non-null reference values x, y, and z, if x.equals(y) returns true and y.equals(z) returns true, then x.equals(z) should return true.
  • It is consistent: for any non-null reference values x and y, multiple invocations of x.equals(y) consistently return true or consistently return false, provided no information used in equals comparisons on the objects is modified.
  • For any non-null reference value x, x.equals(null) should return false.
  • 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

from Object.equals().

So, use the fields needed to fulfill the rules.

rodrigoap
  • 7,405
  • 35
  • 46
4

Well, HomoSapiens#hashcode will be enough with CPerkins's answer.

@Override     
public int hashCode() {     
    int hash = super.hashCode();
    hash = 89 * hash + Objects.hash(name);     
    hash = 89 * hash + Objects.hash(faceBookNickname);     
    return hash;     
}

If you want those parent's fields(gender, weight, height) in action, one way is creating an actual instance of parent type and use it. Thank God, it's not an abstract class.

@Override
public boolean equals(Object obj) {
    if (obj == null) {
        return false;
    }
    if (getClass() != obj.getClass()) {
        return false;
    }
    final HomoSapiens other = (HomoSapiens) obj;
    if (!super.equals(new Hominidae(
        other.gender, other.weight, other.height))) {
         return false;
    }
    if (!Objects.equals(name, other.name)) return false;
    if (!Objects.equals(faceBookNickname, other.faceBookNickname))
        return false;
    return true;
}

I'm adding a way to (I think) solve this. The key point is adding a method loosely checks the equality.

public class Parent {

    public Parent(final String name) {
        super(); this.name = name;
    }

    @Override
    public int hashCode() {
        return hash = 53 * 7 + Objects.hashCode(name);
    }

    @Override
    public boolean equals(final Object obj) {
        return equalsAs(obj) && getClass() == obj.getClass();
    }

    protected boolean equalsAs(final Object obj) {
        if (this == obj) return true;
        if (obj == null) return false;
        if (!getClass().isAssignableFrom(obj.getClass())) return false;
        final Parent other = (Parent) obj;
        if (!Objects.equals(name, other.name)) return false;
        return true;
    }

    private final String name;
}

And here comes the Child.

public class Child extends Parent {

    public Child(final String name, final int age) {
        super(name); this.age = age;
    }

    @Override
    public int hashCode() {
        return hash = 31 * super.hashCode() + age;
    }

    @Override
    public boolean equals(final Object obj) {
        return super.equals(obj);
    }

    @Override
    protected boolean equalsAs(final Object obj) {
        if (!super.equalsAs(obj)) return false;
        if (!getClass().isAssignableFrom(obj.getClass())) return false;
        final Child other = (Child) obj;
        if (age != other.age) return false;
        return true;
    }

    private final int age;
}

Testing...

@Test(invocationCount = 128)
public void assertReflective() {
    final String name = current().nextBoolean() ? "null" : null;
    final int age = current().nextInt();
    final Child x = new Child(name, age);
    assertTrue(x.equals(x));
    assertEquals(x.hashCode(), x.hashCode());
}

@Test(invocationCount = 128)
public void assertSymmetric() {
    final String name = current().nextBoolean() ? "null" : null;
    final int age = current().nextInt();
    final Child x = new Child(name, age);
    final Child y = new Child(name, age);
    assertTrue(x.equals(y));
    assertEquals(x.hashCode(), y.hashCode());
    assertTrue(y.equals(x));
    assertEquals(y.hashCode(), x.hashCode());
}

@Test(invocationCount = 128)
public void assertTransitive() {
    final String name = current().nextBoolean() ? "null" : null;
    final int age = current().nextInt();
    final Child x = new Child(name, age);
    final Child y = new Child(name, age);
    final Child z = new Child(name, age);
    assertTrue(x.equals(y));
    assertEquals(x.hashCode(), y.hashCode());
    assertTrue(y.equals(z));
    assertEquals(y.hashCode(), z.hashCode());
    assertTrue(x.equals(z));
    assertEquals(x.hashCode(), z.hashCode());
}
Jin Kwon
  • 20,295
  • 14
  • 115
  • 184
2

Regarding the accepted @CPerkins answer, I don't think the given equals() code will work reliably, due to the likelihood that the super.equals() method will also check for class equality. A subclass & superclass will not have equal classes.

MattRing
  • 45
  • 1
  • 3
  • 1
    This is not an answer, but a comment. It is a very valid comment though. – Adriaan Koster Jun 15 '16 at 13:20
  • The super.equals() and super.hashcode() of the superclass should be written in such a way that they do work for subclasses and if not they should be final. – Jonathan Rosenne Aug 20 '16 at 15:49
  • 2
    bit late of a reaction, but still: this poses no problem at all. After all: the moment you call the super.equals, you still call it from an instance of the child class. The super class containing a (this.)getClass() doesn't change the fact that method is called on the instance, and the class of that instance doesn't change. – Stultuske Aug 31 '17 at 11:56
  • Ye, voted down :) – Jack Mar 02 '23 at 18:31
2

It's worth noting that the IDE auto generation maybe has taken into consideration super class,just provided that the equals() and hashCode() of super class exists yet. That is, should auto generate these two functions of super first, and then auto generate of the child. I got below right example under Intellj Idea:

@Override
public boolean equals(Object o) {
    if (this == o) return true;
    if (o == null || getClass() != o.getClass()) return false;
    if (!super.equals(o)) return false;

    TActivityWrapper that = (TActivityWrapper) o;

    return data != null ? data.equals(that.data) : that.data == null;
}

@Override
public int hashCode() {
    int result = super.hashCode();
    result = 31 * result + (data != null ? data.hashCode() : 0);
    return result;
}

The problem happens just when you don't auto generate super's in first. Please check above under Netbeans.

Ming
  • 21
  • 2
  • 1
    Nope. Look at the 2nd line of your equals() implementation. If the superclass has the same line (as it should), then the calls to super.equals(o) is going to fail because they aren't of the same type. The subclass really needs to compare ALL properties that define equality, not just locally declared properties. Private fields that aren't accessible even via getter really shouldn't be part of an equality check in the subclass, semantically. Why care about a difference that cannot be seen? – ideasculptor Jun 19 '17 at 19:03
  • super.equals may end to Object#equals which is identity... Just take care with – Max Jan 17 '19 at 16:50
1

Because inheritance breaks encapsulation, subclasses that implement equals() and hashCode() must, necessarily, account for the peculiarities of their superclasses. I've had success encoding calls to the parent class's equals() and hashCode() methods from the subclass's methods.

Steve Emmerson
  • 7,702
  • 5
  • 33
  • 59
  • Similarly, overriding `equals()` and `hashCode()` in `Hominidae` will make the corresponding methods in `HomoSapiens` comparatively easier to implement. – trashgod Jan 14 '10 at 20:12
  • @trashgod:overriding equals() and hashCode() in Hominidae will not make the corresponding methods in HomoSapiens easier to implement because we can not do something like "super.equals(obj.super)" where obj is the compared object ... – wj. Jan 14 '10 at 20:22
  • @wj: You're right. I was thinking of this example, which invokes the corresponding methods of `String`: http://stackoverflow.com/questions/1924728/why-isnt-collections-binarysearch-working-with-this-comparable/1926111#1926111 – trashgod Jan 15 '10 at 01:36
  • @trashgod: in fact I was totally wrong ... and you was absolutely right :) – wj. Jan 15 '10 at 16:22
  • Any suggestion amigo? – Jack Mar 02 '23 at 18:32
1

It sounds like your parent (super) class doesn't override equals. If this is the case then you need to compare the fields from the parent class when you override this method in the sub-class. I agree that using the commons EqualsBuiler is the way to go but you do need to be careful that you don't break the symmetry/transative portions of the equals contract.

If your sub-class adds attributes to the parent class and the parent class isn't abstract and overrides equals you're going to get in to trouble. In this scenario you should really look at object composition instead of inheritance.

I'd strongly recommend the section in Effective Java by Joshua Block on this. It's comprehensive and really well explained.

BigMikeW
  • 821
  • 5
  • 14
0

I believe they now have a method that just does this for you :

EqualsBuilder.reflectionEquals(this, o);

HashCodeBuilder.reflectionHashCode(this);

Terry H
  • 310
  • 4
  • 19
  • 1
    Doing reflection is very expensive and those 2 methods must be implemented in an optimized performance way. – Max Jan 08 '19 at 09:40
  • Obviously, the obvious is obvious. But seriously, I don't see how the reflection done by these methods will be any more expensive in performance than writing the code to do it would be. – Terry H Jan 10 '19 at 18:31
  • 1
    https://stackoverflow.com/questions/435553/java-reflection-performance 1. If you have an hashSet, for example, and you want to add an item, then `hashCode` and `equals` may/are intensively used. 2. if you implement your hashCode and/or equals using reflection, then you may take in account all the object's instance variables. And you may avoid somes (I have an horrible example in mind: an object containing metadata and a file/stream). – Max Jan 11 '19 at 13:51
  • None of that seems to matter though. You can come up with all the horrible scenarios you want, but if you need for your equals or hash functions to take all those things into account you have to take them into account. You can use the built in reflection or write code to do it, but it must be done. I find that it is rare that anyone can write more performant code than what is built into these function. It can be done, sure, but it rarely happens in practice. – Terry H Jan 17 '19 at 14:19