TL;DR version: Don’t implement
equals() on mutable objects.
This is a post I’ve been tossing around for a couple of years, ever since a lunchtime debate with a colleague. It’s a simple statement: You shouldn’t implement the
equals() method if your object isn’t immutable.1
Why? Because when two objects are equal, they should really be considered to be interchangeable. If I’ve got two references to two distinct-but-equal objects, it shouldn’t matter if those references get mixed up – say, by trying to add into a
Set (Do you check the return code when adding to a collection?). You can only do this properly if the objects are immutable – because if they are mutable, then the distinct-but-equal objects can become distinct-and-not-equal over time.
This discussion centred around persistent data (with Hibernate, but that’s not important). I was making the point that you couldn’t rely on the
equals() method to determine if, say, two
Customer objects were equal – because even if they were backed by the same data, the objects could be divergent. This is true even if they have the same version – because they could both be dirty but with different changes. Because of this, there is no reason to implement
equals() on a Hibernate-backed object.2
Instead of implementing
equals() on a mutable object, it’s much better to create smaller, immutable objects (which can implement
equals()), and use those. For example, rather than having a
Orders keyed by
Customer objects, have a
Orders keyed by
CustomerId objects. If you’re using Hibernate, turn these into custom types – that’s a good idea, anyway.
So this becomes really simple: don’t implement the
equals() method if your objects are mutable. Just don’t. Feel free to create lots of smaller types, and compose them upwards. Immutability is a good thing – even if you don’t buy into the functional-programming fad.
Immutable after construction, anyway. Developing full-blown immutable objects can be a bit of a pain, and for code in your own apps, it’s possible to get away with conventions like “only call these methods during construction”. As an example, I’ve seen (and used)
withXYZ()methods, instead of
setXYZ()and returning the object, used in this manner. ↩
- Which makes this piece of advice completely wrong. Admittedly, the first wrong part is re-attaching detached instances, but this just compounds the problem. The fact that it’s the only solution to the problem is just more proof that the problem is an example of the wrong sort of question. ↩