0

I'm trying to implement this:

public boolean equals(Object o) {
    if (o == this) {
        return true;
    }
    if ((null == o) || !(o instanceof Document)) {
        return false;
    }
    Document other = (Document) o;
    // compare in a null-safe manner
    if (list == null) {
        if (other.list != null)
            return false;
    } else if (other.list == null)
        return false;
    else if (!(list.size() == other.list.size())
            && !(list.equals(other.list)))
        return false;
    return true;

where 'list' is a class variable as well as a field of the object 'o'. Please note that the object 'o' has many other fields including booleans and collection too and I need to compare all of them. I tried finding related answers but most of them recommend switch cases or other Java 8 components which is not relevant to my scenario.

  • 3
    Your IDE can do that - generate equals/hashcode – Murat Karagöz Feb 06 '20 at 11:27
  • 2
    Does this answer your question? [Right way to implement equals contract](https://stackoverflow.com/questions/3181339/right-way-to-implement-equals-contract) – default locale Feb 06 '20 at 11:28
  • 2
    In short, use [`Objects.equals(list, other.list))`](https://docs.oracle.com/javase/8/docs/api/java/util/Objects.html#equals-java.lang.Object-java.lang.Object-) – default locale Feb 06 '20 at 11:29
  • 2
    Consider using [Lombok](https://objectcomputing.com/resources/publications/sett/january-2010-reducing-boilerplate-code-with-project-lombok#data). – Boris Feb 06 '20 at 11:33
  • Please take a look https://kodejava.org/how-do-i-implement-equals-method-using-commons-lang/ There is a similar class `HashCodeBuilder` – dimirsen Z Feb 06 '20 at 12:12
  • Why is that (`multiple if else blocks`) is a problem? And, instead of reinventing the wheel, why don't you use the IDE's feature to do it for you? – Arvind Kumar Avinash Feb 06 '20 at 12:15

2 Answers2

1

You're greatly complicating things. Writing "equals" is lengthy boring boilerplate, but you're making it even longer.

public boolean equals(Object o) {
    if (o == this) {
        return true;
    }
    if (!(o instanceof Document)) {
        return false;
    }
    Document other = (Document) o;
    if (!Objects.equals(list, other.list)) {
        return false;
    }
    return true;
}

All you need per reference field are the three lines above, similarly for primitives (don't forget to handle NaN for floating point).

Your condition is not only much longer, but it also lacks symmetry. This makes it much harder to write and more error-prone.

Anyway, writing "equals" is not something you should do often manually. I recommend using Lombok and there are many more tools, e.g., AutoValue or EqualsBuilder.

maaartinus
  • 44,714
  • 32
  • 161
  • 320
  • Thanks for your concise ans. This one looks way better. I'm a beginner in coding and didn't do equals like this ever before. For one of my assigned bugs, I faced this issue where one of the collection fields was returning null when compared with the instance object which was having the values. When I removed .size() method, I was no longer facing NPE. – Md Irfan Mullick Feb 06 '20 at 16:51
  • @MdIrfanMullick You're welcome. Comparing the size is surely an unnecessary complication and every complication may lead to errors. With `Objects.equals`, everything is much simpler. Do not forget to define `hashCode` as well and make sure the two methods fit together. – maaartinus Feb 06 '20 at 19:29
0

A direct rewrite would be:

// compare in a null-safe manner
if (list == null || other.list == null) {
    return list == other.list;
} else {
    return list.size() == other.list.size() ||
           list.equals(other.list));
}

except if the type of list is a standard Java SE List class you can do away with the size() micro-optimization. (A typical `List.equals implementation will do that for you.) So we can rewrite the above as

// compare in a null-safe manner
if (list == null || other.list == null) {
    return list == other.list;
} else {
    return list.equals(other.list));
}

except that that is what Objects.equals(...) does. So the final rewrite is:

// compare in a null-safe manner
return Objects.equals(list, other.list);

It is unclear if an IDE will generate equals methods test the fields in a null-safe way. But the counterpoint to that is that it is advisable to design your classes so that you don't need to do that. For example, use an empty List rather than a null.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216