Super class has wrong implementation of equals and hashcode methods

[Atul More]

Hi,

I have few question:
1. What happen if there is an issue with the implementation of equals and hashcode methods of parent class, in that case how the child class will behave?
2. How to fix the issue? What steps we need to take?


Thanks,
Atul

[Campbell Ritchie]

Depends on how badly wrong the superclass' equals method is. If the hash code method isn't consistent with equals, you can put things into a Collection and not find them again:-

Foo foo = new Foo(...);
Set<Foo> set = new HashSet<>();
set.add(foo);
System.out.printf("The collection %s contain foo.%n",
      set.contains(f) ? "does" : "doesn\u2019t");
for (Foo f : set)
{
   if (f.equals(foo))
   {
      System.out.println("The collection does contain foo.");
   }
}

. . . and you will find it both doesn't and does contain foo.
Also, what is the error in your superclass?

[Atul More]

Hi Campbell,

Thanks for response.
I just gone through an article, where one of the line has stated this situation.
I just wondering, what is the effect.
I was trying to write a code but I guess I missed something.
So I asked this question on this forum, to get some help to understand the situation and way to handle it.

Also in what scenario it goes bad?

Some of my properties come from my parent class, few I have added in child class. I am comparing my child object for lets sya for comparison and it fails, right?

Thanks,
Atul

[Campbell Ritchie]

My pleasure

Actually, I think I rushed a bit with the previous post and missed something out. You may not notice that problem with one Foo instance, and then find it appears if you have two distinct Foo instance for which f1 == f2 evaluates to false, but f1.equals(f2) returns true. I think I shou‍ld have written

Foo f1 = new Foo(...);
Foo f2 = new Foo(...);
Set<Foo> set = new HashSet<>();
set.add(f2);
System.out.printf("The collection %s contain f1.%n",
      set.contains(f) ? "does" : "doesn\u2019t");
for (Foo f : set)
{
   if (f.equals(f1))
   {
      System.out.println("The collection does contain f1.");
   }
}

You are going to have trouble with adding fields to the subtype. You will be in the situation of the coloured point class, which you will find details of in Effective Java by Joshua Bloch (chapter 3). Let's say you have these classes:-

class Car
{
   private Colour colour;
   private String registrationNumber;
   private Maker maker;

   // ...

   @Override
   public int hashCode()
   {
      return ((getClass().getName().hashCode() * 31 +
               colour.hashCode()) * 31 +
               registrationNumber.hashCode()) * 31 +
               maker.hashCode();
   }

   @Override
   public boolean equals(Object ob)
   {
      return ob == this ||
             ob instanceof Car &&
             ((Car)ob).colour.equals(this.colour) &&
             ((Car)ob).registrationNumber.equals(this.registrationNumber) &&
             ((Car)ob).maker.equals(this.maker);
   }
}

Let us assume there is no risk of any of the fields pointing to null. Let us assume that all those fields have correctly‑implemented equals() and hashCode() methods. If those assumptions are correct, you now have correctly‑overridden equals() and hashCode() methods. To go through that equals() method:-

Line 21 tests whether the argument is the same object, because every object is equal to itself.

In line 22, the instanceof test will fail if ob cannot used as an instance of Car, including if ob is null, so there is no need for a separate nullity test.

It shou‍ld be obvious what lines 23‑25 do. Remember they will fail with an Exception if any of ob's fields points to null, but I said there aren't any nulls.

So far, so good: now let's look at this class:-

class Taxi extends Car
{
   private BigDecimal chargePerMile;
   // ...
}

Read about BigDecimal#equals() carefully. Now, how are you going to implement an equals() method? Let's try this:-

@Override
public boolean equals(Object ob)
{
   return super.equals(ob) &&
          ((Taxi)ob).chargePerMile.equals(this.chargePerMile);
}

The maker, colour, and registration number are all taken care of by the super.equals() call, and the charge per mile is taken care of in the new method. So let's have some examples:-

Car c1 = new  Car(Colour.BLACK, "CR 9999", new Maker("Bentley"));
Car c2 = new Taxi(Colour.BLACK, "CR 9999", new Maker("Bentley"),
            new BigDecimal("2.50"));
Car c3 = new Taxi(Colour.BLACK, "CR 9999", new Maker("Bentley"),
            new BigDecimal("2.75"));

Now, this is a situation which shouldn't happen in real life, but we now try c1.equals(c2) etc:-

System.out.println(c1.equals(c2)); // true
System.out.println(c1.equals(c3)); // true
System.out.println(c2.equals(c3)); // false

That breaches the transitivity requirement of the general contract for equals(). So we try something different: we change the superclass' method to read as follows. Note the null test is now essential, and the parts 1‑3 must go in this order, and you must use the && operator not &. Remember that passing null as an argument to equals() shou‍ld always produce the result false.   @Override   public boolean equals(Object ob)   {      return ob == this ||             ob != null &&             ob.getClass().equals(this.getClass()) &&             ((Car)ob).colour.equals(this.colour) &&             ((Car)ob).registrationNumber.equals(this.registrationNumber) &&             ((Car)ob).maker.equals(this.maker);   } }Now the Taxis are in a different class, so your test gives:-

System.out.println(c1.equals(c2)); // false
System.out.println(c1.equals(c3)); // false
System.out.println(c2.equals(c3)); // false

We are now compliant with every requirement of the general contract for equals(), but breaching the Liskov Substitution Principle (=LSP) that you can use a subclass' instance for one of the superclass transparently; note we now have two Cars which shou‍ld be identical returning false from their equals() methods.

Another option: put all those classes (with public access) in the same package, and seal the package so they can only be used outwith that package. Put the new field in the superclass:-

class Car
{
   private Colour colour;
   private String registrationNumber;
   private Maker maker;
   BigDecimal chargePerMile; // not package access: inherited by Taxi
   // ...
}

class Taxi extends Car
{
   // private BigDecimal chargePerMile;
   /* The charge per mile field is now in the superclass */
}

And what value are we going to give the chargePerMile field? Zero? In that case you are going to get false false false again What if you let it be null in the Car class and then say that you only test for differences if the field isn't null? That puts us back where we were with the first example, true true false.

Whichever combinations you try, you will find there are three things you can try to do:-

1: Add fields in a subclass.

2: Maintain the general contract for equals().

3: Maintain the LSP.

It has proven impossible to work out a combination of conditions allowing one to fulfil all three of those requirements. So any fields added to subclasses cannot be used in the equals() method. If you go through the Joshua Bloch reference I told you about, Odersky Spoon and Venners, and search for Angelika Langer Java equals hashCode (be sure to find the English version, not German), you will find all sorts of useful information, and also suggestions about how to sort out the problem of additional fields. You will probably also find this is an insurmountable problem with inheritance, which is one reason why people use inheritance much less than they did maybe twenty years ago.

[João Victor Gomes]

Hello Campbell

Nice explanation. I understand that when we have more than one field to define the equality of the object, we must be careful with the equals and hashcode methods inside the subclasses, but I have a question.
When the object equality will be defined by only one field, for example, an id, and if I have an abstract superclass, where the id field is defined, is it a good idea to override the equals and hashcode inside the super class, and not overriding inside the subclass? Since in this case it will be only possible to create instances of subclasses, and the instance of the subclass will have its own ID, so at runtime the equals and hashcode versions of the superclass would be executed, using the object's id value.
Is this a good way to go?

[Campbell Ritchie]

public abstract class Vehicle
{
   ...
}

public class Car extends Vehicle
{
   ...
}

public class Van extends Vehicle
{
   ...
}

So, Vehicles usually have IDs, called registration numbers or license numbers depending where you are. If two Vehicle instances have the same ID, what is going to happen if you have the equals() method in the Car and Van classes? Should you allow the Van and Car classes to have their own equals() methods?

[David Simkulette]

João Victor Gomes wrote:Hello Campbell

Nice explanation. I understand that when we have more than one field to define the equality of the object, we must be careful with the equals and hashcode methods inside the subclasses, but I have a question.
When the object equality will be defined by only one field, for example, an id, and if I have an abstract superclass, where the id field is defined, is it a good idea to override the equals and hashcode inside the super class, and not overriding inside the subclass? Since in this case it will be only possible to create instances of subclasses, and the instance of the subclass will have its own ID, so at runtime the equals and hashcode versions of the superclass would be executed, using the object's id value.
Is this a good way to go?

If equals really does depend only on a single integer and every instance of an object has a different integer (id) then no two objects will ever be equal, or depending on how you implement things and whether serialization is involved, will ever be equal for the life of the runtime.  So could you define equal in the super class then defer to the super class in the subclass? You could, but since the super class is (obviously) not final how do you know you won't have yet another variant subclass of the superclass with new properties for which this won't work ?

Generally, you want two objects not of the same class to not be equal so an instance of  a superclass (which is not also a subclass) is not equal to any instance of its subclass and the other poster showed you how to check for that in equals.

Most IDEs produce the standard implementation of equals and hashcode on command.

[Campbell Ritchie]

David Simkulette wrote:. . .  another variant subclass of the superclass with new properties . . .

If you try to incorporate additional fields in an implementation of equals() in a subclass, you can be sure to fail to fulfil the general contract for equals(), or the Liskov Substitution Principle, or both.

[João Victor Gomes]

Campbell Ritchie wrote:So, Vehicles usually have IDs, called registration numbers or license numbers depending where you are. If two Vehicle instances have the same ID, what is going to happen if you have the equals() method in the Car and Van classes? Should you allow the Van and Car classes to have their own equals() methods?

If the ID field is defined inside Vehicle class, and it is the only field responsible for defining if an object is equal to another (of course, taking into consideration the other rules, for example, if the object is of the same type of another), the Van and Car classes should not have their own equals() method, to avoid the inconsistence mentioned in your other post. And the ID test inside the equals() method couldn't be done directly inside the subclasses if I make the ID field private, besides, having one equals() version for each of those two classes could lead to the problem of having different fields to define the equality.

David Simkulette wrote:So could you define equal in the super class then defer to the super class in the subclass? You could, but since the super class is (obviously) not final how do you know you won't have yet another variant subclass of the superclass with new properties for which this won't work ?

Hello David, thanks for your answer.
But in my case, I don't plan to have the equals method defined inside the subclasses (for the reasons that Campbell mentioned), and the equality will be defined by only one field, and for my needs, it will work, since it is a simple unique ID test.


Any tips are welcome, thank you all for the nice discussion. I want to know all the possibilities to consider when overriding equals() method.

[Campbell Ritchie]

João Victor Gomes wrote:. . . If the ID field is defined inside Vehicle class . . . the Van and Car classes should not have their own equals() method . . .

I think that answers your question

Three resources worth reading:

1: Effective Java by Joshua Bloch pages 33‑50

2: Odersky Spoon and Venners

3: Angelika Langer

Start with your standard book about equals(), then read those three links. That should get you well on the way to understanding equals().

[João Victor Gomes]

Campbell Ritchie wrote:

João Victor Gomes wrote:. . . If the ID field is defined inside Vehicle class . . . the Van and Car classes should not have their own equals() method . . .

I think that answers your question

Three resources worth reading:

1: Effective Java by Joshua Bloch pages 33‑50

2: Odersky Spoon and Venners

3: Angelika Langer

Start with your standard book about equals(), then read those three links. That should get you well on the way to understanding equals().

Yes, I'm currently studying for the OCP exam using a book focused on the exam, plus the two Core Javas (by Cay Horstmann), and I ordered the book Effective Java, just waiting it to arrive

Thanks for the advices.

[Campbell Ritchie]

João Victor Gomes wrote:. . . Effective Java, just waiting it to arrive

It would be welcome in a 3rd edition with Java89 features in, but it is an excellent book

Thanks for the advices.

That's a pleasure