How to obey contract of equals() when deriving from abstract class

Question

In his book Effective Java, Joshua Bloch writes about the pitfalls that occur with the contract of equals() when derived classes add additional fields to the check. Normally, this would break the symmetry, but Bloch states that "you can add a value component to a subclass of an abstract class without violating the equals contract".

Obviously this is true because there can be no instances of the abstract class, so there is no symmetry to violate. But what about other subclasses? I wrote this example, intentionally omitting hashcode implementations and null-checks to keep the code short:

public abstract class Vehicle {

    private final String color;

    public Vehicle(String color) {
        this.color = color;
    }

    public String getColor() {
        return color;
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;

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

        Vehicle that = (Vehicle) o;

        return color.equals(that.color);
    }

}

public class Bicycle extends Vehicle {

    public Bicycle(String color) {
        super(color);
    }

}

public class Car extends Vehicle {

    private final String model;

    public Car(String color, String model) {
        super(color);
        this.model = model;
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;

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

        Car that = (Car) o;

        return getColor().equals(that.getColor()) && model.equals(that.model);
    }

}

When I create one instance of each class with the same color string, the symmetry of equals() is broken:

Bicycle bicycle = new Bicycle("blue");
Car car = new Car("blue", "Mercedes");

bicycle.equals(car) <- true
car.equals(bicycle) <- false

I am unsure on how to handle this the best way. Declare equals() as abstract in the abstract class to enforce an implementation in the subclasses? But the same effect could be achieved by not declaring equals () at all in the abstract class.

Answer 1

Java's equals contract gets especially spotty in situations like this, and in the end it all becomes a matter of the programmer's preferences and needs. I remember reaching this very same issue and I came across this article, which goes over several possibilities and issues when considering the equals contract with Java. It basically ends up saying there's no way to properly do it without breaking the Java equals contract.

When dealing with abstract classes, my personal preference is to not give the abstract class an equals method at all. It doesn't make sense. You can't have two objects of an abstract type; how should you compare them? Instead, I give each subclass its own equals, and the runtime handles the rest whenever equals() is called. And overall, the solution presented in the article that I most often follow is the "only objects of the exact same class may be compared", which seems the most sensible to me.

Answer 2

Comparing the class object instead of doing an instanceof check solves the problem.

        if (getClass() != obj.getClass()) {
            return false;
        }

Here is the full implementation (generated by Eclipse):

public class Vehicle {

    // ...    

    @Override
    public boolean equals(Object obj) {
        if (this == obj) {
            return true;
        }
        if (obj == null) {
            return false;
        }
        if (getClass() != obj.getClass()) {
            return false;
        }
        Vehicle other = (Vehicle) obj;
        if (color == null) {
            if (other.color != null) {
                return false;
            }
        } else if (!color.equals(other.color)) {
            return false;
        }
        return true;
    }

}

public class Car extends Vehicle {

    // ...

    @Override
    public boolean equals(Object obj) {
        if (this == obj) {
            return true;
        }
        if (!super.equals(obj)) {
            return false;
        }
        if (getClass() != obj.getClass()) {
            return false;
        }
        Car other = (Car) obj;
        if (model == null) {
            if (other.model != null) {
                return false;
            }
        } else if (!model.equals(other.model)) {
            return false;
        }
        return true;
    }


}

Both checks in your example will then yield false.

Answer 3

The symmetry of the equals() got broken mainly, because Bicycle class is a subclass and it is dependent on the super class (Vehicle) for it's own equality. If you define the equals() method for each sub class then you will not encounter this issue.

Here is the equals() implementation for each of the class. (Only Bicycle equals() is added, other implementations are same but simplified.)

public abstract class Vehicle {
....
      @Override
  public boolean equals(Object o) {
    if (this == o) return true;
    if (!(o instanceof Vehicle)) return false;
    Vehicle that = (Vehicle) o;
    return color.equals(that.color);
  }
}

public class Bicycle extends Vehicle {
...
  @Override
  public boolean equals(Object o) {
    if (this == o) return true;
    if (!(o instanceof Bicycle)) return false;
    Bicycle that = (Bicycle) o;
    return super.getColor().equals(that.getColor());
  }
}

public class Car extends Vehicle {
...
  @Override
  public boolean equals(Object o) {
    if (this == o) return true;
    if (!(o instanceof Car)) return false;
    if (!super.equals(o)) return false;
    Car car = (Car) o;
    return model.equals(car.model);
  }
}

// This is main class for testing the above functionality.

class MainClass {
  public static void main(String[] args) {
    Bicycle bicycle = new Bicycle("blue");
    Car car = new Car("blue", "Mercedes");

    System.out.println(bicycle.equals(car));   -> false
    System.out.println(car.equals(bicycle));   -> false
  }
}

---

OR you should use the object.getClass() instead of instanceof operator in your super class implementation as suggested by @FranzBecker. The sub classes can still use the instanceOf operator without any issue.

public abstract class Vehicle {
...
  @Override
  public boolean equals(Object o) {
    if (this == o) return true;
    if ((this.getClass() !=  o.getClass())) return false;
    Vehicle that = (Vehicle) o;
    return color.equals(that.color);
  }
}