how do I correctly override equals for inheritance in java?

Question

I am using hibernate and id... is used for persistence (which is why it is omitted in comparison). (Also, using google guava helper equals)

HolidayPackageVariant:

public abstract class HolidayPackageVariant {
    private Integer idHolidayPackageVariant;
    private HolidayPackage holidayPackage;
    private String typeHolidayPackage;

@Override
public boolean equals(Object obj) {
    if (obj == this)
        return true;
    if(obj == null)
        return false;

    if (getClass().equals(obj.getClass())) {
        final HolidayPackageVariant otherPackageVariant = (HolidayPackageVariant) obj;
        return Objects.equal(getTypeHolidayPackage(),otherPackageVariant.getTypeHolidayPackage())
                && Objects.equal(getHolidayPackage(),
                        otherPackageVariant.getHolidayPackage());
    }
    return false;
}

@Override
public int hashCode() {
    return Objects.hashCode(getTypeHolidayPackage(), getHolidayPackage());
}

FlightHolidayPackageVariant:

public final class FlightHolidayPackageVariant extends HolidayPackageVariant{
    private Destination originCity;
    public boolean equals(Object obj) {
    // ..

Should I completely override the equals() or should I be invoking super.equals(...) in some way ?

[Secret of equals](http://www.angelikalanger.com/Articles/JavaSolutions/SecretsOfEquals/Equals.html); [Effective Java](https://docs.google.com/viewer?a=v&q=cache:qjZ_JbpnXCUJ:java.sun.com/developer/Books/effectivejava/Chapter3.pdf+effective+java+equals&hl=en&gl=uk&pid=bl&srcid=ADGEESith0oQAHumur5q4botdHMQ2NK7pVUOZBXXeQk_Pcu-mh2V0hXFmDkyE3H50j5RtvvTWcqjq62EaK27r9Y0UzQDTaOXYQp2mrYJNCngyeRblQ4taQx4tucpV1He2BSkI8zLRZM5&sig=AHIEtbR9cAUIlgBoIS1NqH5eCPPR-7HitA) — McDowell, Mar 27 '12 at 08:47
I wonder why don't you use helper classes EqualsBuilder and HashCodeBuilder from the Apache Commons Lang library? — aviad, Mar 27 '12 at 09:52
@aviad After doing some research, I learnt that it used reflection and was quite slow compared to google Guava's equals helper (which is what I am using in this example also). See `Objects.equal(...)` — brainydexter, Mar 27 '12 at 09:56
just found this question that does great comparison between Guava and Apache Commons: http://stackoverflow.com/questions/4542550/what-are-the-big-improvements-between-guava-and-apache-equivalent-libraries — aviad, Mar 27 '12 at 10:13
@aviad Long story short, my biggest rant against Commons is that it uses reflection which makes it slow. +1 for the link — brainydexter, Mar 27 '12 at 10:35
Are you sure you want that FlightHolidayPackageVariant extends the class HolidayPackageVariant? If you make it a composition of both HolidayPackageVariant and Destination, you can have a more easy way with equals and you can use several good patterns, such as immutability for instance. Hibernate can also manage it more easily if you generate your database schema from your code. — Olivier Grégoire, Mar 27 '12 at 11:27
@ogregoire I have different type of variants, Flight/Land/Hotel etc. Can you give me an example of what advantage I have with composition ? — brainydexter, Mar 27 '12 at 11:32
I don't really know your complete use case, so I'm not going further than advise you to carefully read items 16 to 18 of Effective Java (2nd edition). It states that composition is better than inheritance in most of the cases, but if you must, make sure to properly make your classes inheritable. — Olivier Grégoire, Mar 27 '12 at 12:10
@ogregoire the equals issue is examined more fully in Item 8. — Kevin Bourrillion, Mar 27 '12 at 14:42
@Kevin Bourillion: My previous answer was linked to the question in the comment just above. — Olivier Grégoire, Mar 27 '12 at 18:42

score 3 · Answer 1 · answered Mar 28 '12 at 09:38

This is how you should implement data objects: no inheritance allowed, composition only.

These are immutable objects, but you could want to modify them, so remove final where needed. Also you may remove final from the class definition because Hibernate doesn't support it, but in that case, you should document that these classes are not eligible for inheritance.

The main advantages are all the ones described in Effective Java plus, in this case, you don't have to manage inheritance through Hibernate which can sometimes be a real pain.

public final class HolidayPackageVariant {
  private final Integer idHolidayPackageVariant;
  private final HolidayPackage holidayPackage;
  private final String typeHolidayPackage;

  ...

  @Override
  public boolean equals(Object obj) {
    if (obj == this)
      return true;
    if (!(obj instanceof HolidayPackageVariant))
      return false;

    HolidayPackageVariant that = (HolidayPackageVariant) obj;
    return Objects.equal(this.typeHolidayPackage, that.typeHolidayPackage)
        && Objects.equal(this.holidayPackage, that.holidayPackage);
  }

  @Override
  public int hashCode() {
    return Objects.hashCode(this.typeHolidayPackage, this.holidayPackage);
  }
}

public final class FlightHolidayPackageVariant {
  private HolidayPackageVariant holidayPackageVariant;
  private Destination originCity;

  ...

  public HolidayPackageVariant asHolidayPackageVariant() {
    return this.holidayPackageVariant;
  }

  public boolean equals(Object obj) {
    if (obj == this)
      return true;
    if (!(obj instanceof FlightHolidayPackageVariant))
      return false;

    FlightHolidayPackageVariant that = (FlightHolidayPackageVariant) obj;
    return Objects.equal(this.holidayPackageVariant, that.holidayPackageVariant)
        && Objects.equal(this.originCity, that.originCity);
  }

  @Override
  public int hashCode() {
    return Objects.hashCode(this.holidayPackageVariant, this.originCity);
  }
}

I've been thinking about this. One question that comes to my mind is, in my HolidayPackage, I store a `Set variants;` to point to concrete implementations of HolidayPackageVariant i.e. `LandHolidayPackageVariant` and `FlightHolidayPackageVariant`. If I change it to composition then how do I store references to `FlightHolidayPackageVariant` in that set ? — brainydexter, Mar 28 '12 at 09:59
Do you really need to put them all in a single collection? Can't you use several collections that you can aggregate when needed using Guava's `Iterables.transform(Iterable, Function)` ? — Olivier Grégoire, Mar 28 '12 at 11:08

brainydexter · Answer 2 · 2012-03-27T19:26:30.570

2

Following the Secret of Equals:

HolidayPackageVariant:

public abstract class HolidayPackageVariant {
    private Integer idHolidayPackageVariant;
    private HolidayPackage holidayPackage;
    private String typeHolidayPackage;

    @Override
    public boolean equals(Object obj) {
        if (obj == this) return true;
        if(obj == null) return false;

        if (getClass().equals(obj.getClass())) {
            final HolidayPackageVariant otherPackageVariant = (HolidayPackageVariant) obj;
            return Objects.equal(getTypeHolidayPackage(),otherPackageVariant.getTypeHolidayPackage())
                    && Objects.equal(getHolidayPackage(),
                            otherPackageVariant.getHolidayPackage());
        }
        return false;
    }
}

FlightHolidayPackageVariant:

public final class FlightHolidayPackageVariant extends HolidayPackageVariant{
    private Destination originCity;

    @Override
    public boolean equals(Object obj) {

        if(super.equals(obj)){
            return Objects.equal(getOriginCity(),
                    ((FlightHolidayPackageVariant)(obj)).getOriginCity());
        }
        return false;
    }
}

This will ensure that only the same Type of variants are equal to each other.

edited Mar 27 '12 at 19:26

answered Mar 27 '12 at 11:21

brainydexter

19,826
28
77
115

In `FlightHolidayPackageVariant`, the two lines `if(obj==this) return true; if(obj==null) return false;` are completely unnecessary as these tests are the first tests performed in the `super.equals(obj)`. – Olivier Grégoire Mar 27 '12 at 12:39
This link seems to completely ignore the problem of substitutability, which is important. The author even quotes Effective Java and so it seems like she discovered something EJ didn't already, but EJ already considers her solution and refutes it. – Kevin Bourrillion Mar 27 '12 at 14:44
1

@KevinBourrillion Can you talk a little bit about substitutability that you mentioned ? Also, please suggest me edits in this solution I came up with. – brainydexter Mar 27 '12 at 19:22
You can read about the [Liskov substitution principle on Wikipedia](http://en.wikipedia.org/wiki/Liskov_substitution_principle). – Olivier Grégoire Mar 28 '12 at 08:35

how do I correctly override equals for inheritance in java?

2 Answers2

Linked