CODEWITHSUNDEEP
javainheritanceequals
Kuba Spatny
Kuba Spatny

Reputation: 26990

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

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?

Upvotes: 5

Views: 20887

Answers (4)

Matt
Matt

Reputation: 186

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.

Upvotes: 4

Floegipoky
Floegipoky

Reputation: 3273

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.

Upvotes: 3

peter.petrov
peter.petrov

Reputation: 39477

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.

Upvotes: 0

Kuba Spatny
Kuba Spatny

Reputation: 26990

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.

Upvotes: 1

Related Questions