How not to use (pattern matching with) instanceof

Tags:

Could doesn’t mean should (and sometimes you shouldn’t)

(This post got updated. Please scroll down.)

In the chase for non-trivial examples to illustrate possible usage of JEP 305 (described by me here), some people might have gone too far, I guess. In particular into the more challenging area of the equals() method, present in all Java objects. Let’s do something, which makes some of my fellow developers truly shocked: let’s RTFM of the Object.equals() method. We can read there which conditions must be satisfied by a correct implementation of the equals() method. One of them is symmetric: having any two objects x and y, calling x.equals(y) and y.equals(x) must have the same result. Both have to be true or both have to be false. If one of them is true and the other is false, then the equals() method hasn’t been implemented correctly. Deal with that.

One of the myths of the programming world is that “inheritance is BAD”. (Having enough time I talk about this in CONTEXTVS, STVLTE!)

Inheritance and type hierarchies aren’t bad per se. I really can’t tell how someone can never ever use the inheritance and call it ‘object oriented programming’. What is bad is the clueless usage of the inheritance when it shouldn’t be used, because e.g. it violates the Liskov Substitution Principle. Add mindless usage of the JEP 305 to that, and we have a recipe for a disaster.

All right, what do you mean?

Let’s take a look at this code (an excerpt of this one):

 1class Point {
 2	public final int x;
 3	public final int y;
 4
 5	@Override
 6	public boolean equals(Object o) {
 7		if (this == o) return true;
 8		if (o instanceof Point that) {
 9			return this.x == that.x && this.y == that.y;
10		}
11		return false;
12	}
13	// ...
14}

It looks innocent at the first glance, right? Can we push that? Few weeks pass, and an innocent Junior comes in, who ‘only needs to implement Point, but in 3D, you see’:

 1class Point3D extends Point {
 2	public final int z;
 3
 4	@Override
 5	public boolean equals(Object o) {
 6		if (this == o) return true;
 7		if (o instanceof Point3D that) {
 8			if (!super.equals(o))
 9				return false;
10			return z == that.z;
11		}
12		return false;
13	}
14    //...
15}

It may look innocent at the first glance too… However, running a simple check of correctness of the equals() method:

1var point = new Point (1,1);
2var point3D = new Point3D(1,1,1);
3System.out.println(point.equals(point3D));
4System.out.println(point3D.equals(point));

yields:

true
false

The conclusion is simple: the implementation of the equals() methods is not symmetric. Oh my, but why, oh my? Well, JEP 305 is IMHO quite like any other technology: it should be used correctly, not misused, like in this very case. The mistake is in line 8. ‘But hey, it’s just using pattern matching with instanceof, it was made for this purpose, don’t you know?’ Well, I don’t think so. Point3D is not “ah, you see, just an ordinary point, only one more axis” and it’s incorrect to compare in equals everything that is a point. Yeap, the instanceof yields true not only for objects of exactly the same type, but also for every type, which inherits. Yeap, I know, it’s truism. But boy, I’ve seen too much code to know that this truism isn’t as widely known as it should be…

Loosen your tie, please

In real life (not tutorial one) we need to be pragmatic. Sometimes implementing equals() using instanceof and JEP 305 isn’t bad, if we all in the team are aware of the consequences, we will never inherit from this type, and we need to check identity this way, because some frameworks spawn inheriting proxy classes, e.g. from our entities.

So let me ask you this, please: when you see somewhere in a code (production or blog) instanceof inside the equals() method, be aware. And don’t copy & paste random code from the Internet. Context, stupid!

The update

To make things clear and avoid some misunderstanding, I’ve decided to follow advice from Nicolai Parlog to update this entry in April 2022.

Using instanceof in equals() is not problematic itself. Also using inheritance in Java isn’t problematic itself. Just as using a hammer is not. But the misuse is.

Trouble comes when these are misused, especially together: equals() relying on instanceof plus child classes.

The remedy to this is quite simple: ideally make the class final. Point3D isn’t “a Point with just one more extra dimension/property”. sealed class would also do, provided all classes have correct equals, I guess. If you can’t make the class final, then: public final boolean equals(Object o) is the way to go. By tossing final on equals() we at least make sure that there will be no asymmetrical behaviour of equals.

  • ‘Hold on, but then my Point3D(1, 1, 1) will be equal to Point(1, 1), that’s not what I want, because this point3D isn’t equal to the point!!11one’
  • ‘Exactly, didn’t I tell you Point3D isn’t “a Point with just one more extra dimension/property”? ;-)

TL;DR: finalising makes instanceof in equals safe. TL;DR2: don’t copy-paste random snippets from the internet without understanding the context, please.

​