I am, specifically concerned with obeying the symmetry part of the general contract established in Object#equals(Object)
where, given two non-null objects x
and y
, the result of x.equals(y)
and y.equals(x)
should be the same.
Suppose you have two classes, PurchaseOrder
and InternationalPurchaseOrder
where the latter extends the former. In the basic case, it makes sense that comparing an instance of each against the other shall return consistently false
for x.equals(y)
and y.equals(x)
simply because a PurchaseOrder
is not always an InternationalPurchaseOrder
and therefore, the additional fields in InternationalPurchaseOrder
objects will not be present in instances of PurchaseOrder
.
Now, suppose you upcast an instance of InternationalPurchaseOrder
.
PurchaseOrder order1 = new PurchaseOrder();
PurchaseOrder order2 = new InternationalPurchaseOrder();
System.out.println("Order 1 equals Order 2? " + order1.equals(order2));
System.out.println("Order 2 equals Order 1? " + order2.equals(order1));
We already established that the result should be symmetric. But, should the result be false
for cases when both object contain the same internal data? I believe the result should be true
regardless of the fact that one object has an extra field. Since by upcasting order2
, access to fields in InternationalPurchaseOrder
class is restricted, the result of the equals()
method shall be the result of the call to the super.equals(obj)
.
If all I stated is true, the implementation of the equals
method in InternationalPurchaseOrder
should be something like this:
@Override
public boolean equals(Object obj) {
if (!super.equals(obj)) return false;
// PurchaseOrder already passed check by calling super.equals() and this object is upcasted
InternationalPurchaseOrder other = (InternationalPurchaseOrder)obj;
if (this.country == null) {
if (other.country != null) return false;
} else if (!country.equals(other.country)) return false;
return true;
}
Assuming that country
is the only field declared in this subclass.
The problem is in the super-class
@Override
public boolean equals (Object obj) {
if (this == obj) return true;
if (obj == null) return false;
if (getClass() != obj.getClass()) return false;
PurchaseOrder other = (PurchaseOrder) obj;
if (description == null) {
if (other.description != null) return false;
} else if (!description.equals(other.description)) return false;
if (orderId == null) {
if (other.orderId != null) return false;
} else if (!orderId.equals(other.orderId)) return false;
if (qty != other.qty) return false;
return true;
}
Because the two instances are not of the same class. And if instead I use getClass().isAssignableFrom(Class)
, symmetry is lost. The same is true when using instanceof
. In the book, Effective Java, Joshua Bloch indirectly warn about overriding equals
unnecessarily. In this case, however, it is necessary for the subclass to have an overridden equals
for instances to compare the fields declared in the subclass.
This has gone long enough. Is this a case for a more complicated implementation of the equals
function, or is this simply a case against upcasting?
CLARIFICATION: This question is not answered by the suggestions proposed by @Progman in the comments section below. This is not a simple case of overriding equals
methods for subclasses. I think the code I posted here shows that I have done this correctly. This post is SPECIFICALLY about the expected result of comparing two objects when one is upcasted so that it behaves like an object of the super-class.
It turns out, the problem is a bit deeper than I originally thought for one main reason: Liskov Subsitution Principle.
My PurchaseOrder#equals(Object)
method violates LSP. Because InternationalPurchaseOrder
extends PurchaseOrder
, it is correct to say that an international purchase IS-A purchase order. Therefore, to comply with LSP, I should be able to replace one instance for the other without any ill effects. Because of this, the line if (getClass() != obj.getClass()) return false;
completely violates this principle.
Even when my focus was around upcasting (which by now I am pretty much convinced that it is a code smell), almost the same issue applies: Should I be able to compare instances of PurchaseOrder
and InternationalPurchaseOrder
and return true
if they contain the same data internally? According to Joshua Bloch book Effective Java, Item 10 (Obey the general contract when overriding equals), the answer should be yes, but it is indeed no; but not for the reasons I initially thought (they are not the same type). The problem is this, and I quote:
There is no way to extend an instantiable class and add value component while preserving the
equals
contract, unless you're willing to forgo the benefits of object-oriented abstraction.
He continues to get in more details about this. The conclusion is simple: Do it this way and loose the benefits of abstraction, and violate LSP in the process. Or... simply follow a very important programming principle (especially in Java): favor composition over inheritance and inject the attributes you need to make a PurchaseOrder
domestic or international in this example. In my case, this solution will look like this (irrelevant details of class omitted):
public class InternationalPurchaseOrder {
private PurchaseOrder po;
private String country;
public (PurchaseOrder po, String country) {
this.po = po;
this.country = country;
}
public PurchaseOrder asPurchaseOrder() {
return po;
}
@Override
public boolean equals (Object obj) {
if (!(obj instanceof InternationalPurchaseOrder)) {
return false;
}
InternationalPurchaseOrder ipo = (InternationalPurchaseOrder)obj;
return ipo.po.equals(po) && cp.country.equals(country);
}
}
This way, an InternationalPurchaseOrder
can continue to be compared to other instances of the same class AND if needed to be compared to objects of PurchaseOrder
type, all that is needed is to call the asPurchaseOrder
to return an object of the compatible type.