this question is specifically about the performance and to some extent brevity of the various implementation alternatives.
I refreshed myself with this article on implementing equality right. My question particularly corresponds to canEqual
(to ensure equivalence relation).
instead of overloading canEquals method to use instanceOf in every class in the hierarchy( instance of paramenter is a compile time class ). Why not use isAssignableFrom ( which is resolved dynamically ) in only the top level class. Makes for much concise code and you dont have to overload a third method.
While, this alternative works. Are there any performance considerations that I need to be aware of?
enum Color {
RED, ORANGE, YELLOW, GREEN, BLUE, INDIGO, VIOLET;
}
class Point {
int x;
int y;
public Point(int x, int y) {
this.x = x;
this.y = y;
}
@Override public boolean equals(Object other) {
boolean result = false;
if (other instanceof Point) {
Point that = (Point) other;
//Option 1
//result = (that.canEqual(this) && this.getX() == that.getX() && this.getY() == that.getY());
//Option 2
//result = (that.getClass().isAssignableFrom(this.getClass()) && this.getX() == that.getX() && this.getY() == that.getY());
//Option 3
//result = (getClass() == that.getClass() && this.getX() == that.getX() && this.getY() == that.getY());
}
return result;
}
@Override public int hashCode() {
return (41 * (41 + x) + y);
}
public boolean canEqual(Object other) { return (other instanceof Point); }
}
public class ColoredPoint extends Point{
Color color;
public ColoredPoint(int x, int y, Color color) {
super(x, y);
this.color = color;
}
@Override public boolean equals(Object other) {
boolean result = false;
if (other instanceof ColoredPoint) {
ColoredPoint that = (ColoredPoint) other;
result = (this.color.equals(that.color) && super.equals(that));
}
return result;
}
@Override public int hashCode() {
return (41 * super.hashCode() + color.hashCode());
}
@Override public boolean canEqual(Object other) { return (other instanceof ColoredPoint); }
public static void main(String[] args) {
Object p = new Point(1, 2);
Object cp = new ColoredPoint(1, 2, Color.INDIGO);
Point pAnon = new Point(1, 1) {
@Override public int getY() {
return 2;
}
};
Set<Point> coll = new java.util.HashSet<Point>();
coll.add((Point)p);
System.out.println(coll.contains(p)); // prints true
System.out.println(coll.contains(cp)); // prints false
System.out.println(coll.contains(pAnon)); // prints true
}
}
Update: Actually, your method is not technically valid like I first thought, because it breaks the symmetry contract of equals
for subclasses that don't override equals
:
Point p = new Point(1, 2);
Point pAnon = new Point(1, 1) {
@Override public int getY() {
return 2;
}
};
System.out.println(p.equals(pAnon)); // prints false
System.out.println(pAnon.equals(p)); // prints true
The reason is that p.getClass().isAssignableFrom(pAnon.getClass())
is true
while the inverse, pAnon.getClass().isAssignableFrom(p.getClass())
is false
.
If you are not convinced by this, try actually running your code and compare it to the version in the article: you will notice that it prints true, false, false
, instead of true, false, true
like the example in the article.
unless you want to allow comparing classes of different types, the easiest, safest, most concise and probably most efficient impl is:
(getClass() == that.getClass())
All the answers given so far don't answer the question - but point out the equals()
contract. Equality must be an equivalence relation (transitive, symmetric, reflexive) and equal objects must have the same hash code. That extends perfectly fine to subclasses - provided subclasses don't themselves override equals()
or hashCode()
. So you have two choices - you either inherit equals()
from Point
(so ColoredPoint
instances are equal if they have the same coordinates, even if they have a different color), or you override equals()
(and now must make sure a Point
and a ColoredPoint
are never equal).
If you need to perform a pointwise comparison, then don't use equals()
- write a method pointwiseEquals()
instead.
Whatever you choose to do, you still have to perform the class check in equals()
.
getClass() == that.getClass()
is clearly the best performer, but it does break if you expect to be able to equality test subclasses that don't themselves override equals()
(and in practice, the only way you can guarantee that is to make the class or the equality methods final and not allow any subclasses to override at all). If it's a choice between instanceOf
and isAssignableFrom
, there's no practical difference, they both in fact perform the same run-time test (the only difference is, instanceOf
can perform a compile-time sanity check, but in this case, it can't know anything when the input is just Object
). In both cases, the runtime check is identical - check for the target class in the object's listed interfaces (which doesn't apply here, since we're not checking for an interface), or walk up the class hierarchy until we either find the listed class or get to the root.
See my answer for What is the difference between equality and equivalence?.
You can't equate two objects from different classes because it breaks symmetry.
Edit:
It comes down to whether x
in the following:
if (other instanceof Point) {
Point that = (Point) other;
boolean x = that.getClass().isAssignableFrom(this.getClass());
}
has the same power as getClass() == that.getClass()
.
According to @waxwing's answer it doesn't.
Even if it were correct, I don't see any performance benefit here by calling that.getClass().isAssignableFrom
.
Here's my Second Answer to the clarified question
Consider when we call Point.equals(ColoredPoint cp);
Point.equals() first checks for
if (other instanceof Point)...
Which passes. Out of the three options presented, all three of them check that the other object, in this case a ColoredPoint, satisfies some more test. The options are:
From a performance (and design) perspective, there was no value in checking for other instanceof Point
, because the actual behavior OP wants (which he has been unable to express) is that for his particular use case, equality between these Objects means they must be the same class.
Therefore, for both performance and design, just use
this.getClass() == that.getClass()
as was suggested by @jthalborn
When a later coder sees instanceof or isAssignableFrom in your code, he will think that subclasses are allowed to equal the base class, which is completely misleading.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With