Inheritance - using super.equals() in subclasses that override methods used in equals of superclass

Question

I've been testing a code and stumbled across a problem: Should you call super.equals() method in subclass that can override some of the methods used in equals() method of the super class?

Let's consider the following code:

public abstract class Item {
    private int id;
    private float price;

    public Item(int id, String name, float price, String category) {
        this.id = id;
        this.name = name;
        this.price = price;
        this.category = category;
    }

    public int getID() {
        return id;
    }

    public float getPrice() {
        return price;
    }

    @Override
    public boolean equals(Object object){
        if(object instanceof Item){
            Item item = (Item) object;
            if( id == item.getID()
                && price == item.getPrice())                    
            { return true; }
        }
        return false;
    }
}

And the subclass DiscountedItem:

public class DiscountedItem extends Item {
    // discount stored in %
    private int discount;

    @Override
    public boolean equals(Object object) {
        if(object instanceof DiscountedItem){
            DiscountedItem item = (DiscountedItem) object;
            return (super.equals(item)
                    && discount == item.getDiscount()
            );
        }
        return false;
    }

    public int getDiscount() {
        return discount;
    }

    @Override
    public float getPrice() {
        return super.getPrice()*(100 - discount);
    }    
}

I've been just re-reading Angelika Langer's secrets of equals(), where she even states:

There is agreement that super.equals() should be invoked if the class has a superclass other than Object.

But I think it's highly unpredictable when the subclass will override some of the methods. For instance when I compare 2 DiscountedItem objects using equals, the super method is called and item.getPrice() is dynamically dispatched to the correct method in the subclass DiscountedItem, whereas the other price value is accessed directly using variable.

So, is it really up to me (as I should implement the method correctly) or is there a way around it?

Your subclass' `equals` method should be implemented in a manner that is consistent with the contract of that method. The reason to delegate to `super.equals()` is that the child class probably can't access fields of the parent that `super.equals()` may depend on. If you violate encapsulation, all bets are off. If you implement your `super.equals()` poorly by enabling a child class to break the contract of its parent, then again, all bets are off. — Floegipoky, Nov 25 '13 at 23:12
One could argue that the root problem here is embedding business logic in a bean getter, and not the more academic question asked :) — Affe, Nov 25 '13 at 23:14
@Affe Oh this isn't even a real web application, it's just an academic example given in software testing class :) But I don't think this error was made on purpose.. — Kuba Spatny, Nov 25 '13 at 23:20
[This](http://stackoverflow.com/questions/2066917/overriding-equals-hashcode-in-sub-classes-considering-super-fields?rq=1) is relevant — Floegipoky, Nov 25 '13 at 23:39

Matt · Accepted Answer · 2013-11-25T23:56:01.717

Compare instance variables directly rather than comparing an instance variable to its related getter method.

For example, change

&& price == item.getPrice())

to

&& this.price == item.price)

The getter method is unnecessary since private instance variables are only inaccessible outside the class structure.

Note:

I previously recommended the following:

&& this.getPrice() == item.getPrice())

While it will work in the question's example, it is not well suited for all cases. Consider if the subclass DiscountedItem declared the method getPrice as such:

@Override
public float getPrice() {
    return Math.floor(super.getPrice());
}

This would result in the false equivalence:

DiscountedItem firstItem = DiscountedItem(1, "", 1.1, "");
DiscountedItem secondItem = DiscountedItem(1, "", 1.0, "");
firstItem.equals(secondItem); // Returns true despite different prices.

I agree with @Floegipoky. The member variables of the class should be compared directly versus comparing the result of getter methods. It is possible to have two objects be equal when comparing their methods even when their variable values are not equal. I will update my answer accordingly. — Matt, Nov 25 '13 at 23:42

score 2 · Answer 2 · answered Nov 25 '13 at 23:28

The implementation of equals should depend on state and type, but not functionality. You went wrong in your base class:

@Override
public boolean equals(Object object){
    if(object instanceof Item){ // Type check- good!
        Item item = (Item) object;
        if( id == item.getID()  // Depends on functionality- bad!
            && price == item.getPrice()) // Depends on functionality- bad!                    
        { return true; }
    }
    return false;
}

item.getID() and item.getPrice(), as you've noticed, can be overwritten to break the contract of Item.equals().

@Override
public boolean equals(Object object){
    if(object instanceof Item){ // Type check- good!
        Item item = (Item) object;
        if( id == item.id  // Depends on state- good!
            && price == item.price)  // Depends on state- good!
        { return true; }
    }
    return false;
}

This will never be broken by a child class. Further, it enables the child to meaningfully delegate to it.

@Override
public boolean equals(Object object) {
    if(object instanceof DiscountedItem){
        DiscountedItem item = (DiscountedItem) object;
        return (super.equals(item)
                && this.discount == item.discount
        );
    }
    return false;
}

The child only needs to worry about comparing the data that it owns.

Thanks for this answer, you are of course right! Usually, when I write the code I do it like you suggest, however I didn't see how wrong it could be until now.. But just for the record, I didn't write this code, I was just supposed to test it. Thank you again! — Kuba Spatny, Nov 26 '13 at 10:38

score 1 · Answer 3 · answered Nov 25 '13 at 23:10

Oh my, I guess I just had to post the question to get it..

To get rid of the problem, instead of directly accessing variables - call the getters!

@Override
public boolean equals(Object object){
      if(object instanceof Item){
          Item item = (Item) object;
          if( this.getID() == item.getID()
              && this.getPrice() == item.getPrice())                    
          { return true; }
      }
      return false;
}

This code no longer has the problem when overriding methods.

score 0 · Answer 4 · answered Nov 25 '13 at 23:10

0

If you call the equals of the DiscountedItem when can you have a problem? If something is DiscountedItem and something else is Item these two are never equal as per equals. Right? So I don't see a problem if you always call the getters.

Also, you need to override hashCode if you override equals.

answered Nov 25 '13 at 23:10

peter.petrov

38,363
16
94
159

I am not trying to compare different objects, but compare parts of objects that were inhereted from their parent. hashCode is implemented, but I didn't include it as it has nothing to do with the problem.. – Kuba Spatny Nov 25 '13 at 23:12
Right, yeah, my point was that these "parts of objects" are always "parts of the objects from the descendant class", so I was saying I see no issue with that (with respect to overriding methods). – peter.petrov Nov 26 '13 at 12:26

Inheritance - using super.equals() in subclasses that override methods used in equals of superclass

4 Answers4