javaliskov-substitution-principleupcastingobject-comparison

What should be the result of overridden "Object#equals(Object)" when instance is upcasted?


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.


Solution

  • 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 InternationalPurchaseOrdercan 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.