Hi Gregor,

> > What it can't figure out is whether the
> > other object is an instance of a subclass that overrode
> > equals
> If could find out if the other class overrode equals -
> using reflection.
> This could very well be the task of a library method like
> pojomatic mentioned above.
>
In practice I expect that would be too slow. Premature optimization, perhaps, but that's my guess. Also, I could tell through reflection if it is a subtype that overrode equals, but I wouldn't be able to tell whether the overridden equals is incompatible with symmetry. If the programer followed the recipe, then it would use instanceof and indeed be incompatible with symmetry, but that couldn't really be assumed.

Hi Daniel,

> What exactly are we attempting to solve with this? Using
> the most implementation-y form of inheritance to very
> slightly reduce some minor code duplication? All so we can
> have Point3D extend Point2D or Circle extend Ellipse?
>
> Is that very slight gain really worth this very real
> cost?
>
> (That said, it's a clever technique.)
>
The main point of the article is to try and spread the word more on the pitfalls of writing equals, and how to avoid them. Everything but the canEqual technique was already conventional wisdom, I think, but not necessarily widespread wisdom. So it doesn't hurt to remind folks.

The point of canEqual is simply to show how to do something that I think was not conventional wisdom. The conventional wisdom, such as in Effective Java, is that this was not possible. I don't have a good use case off the top of my head, though.

Hi Daniel,

> What exactly are we attempting to solve with this? Using
> the most implementation-y form of inheritance to very
> slightly reduce some minor code duplication? All so we can
> have Point3D extend Point2D or Circle extend Ellipse?
>
Somehow I didn't notice your comment about reducing code duplication. The problem wasn't code duplication, and I'm not sure which code you were thinking was being duplicated. The problem was that equals methods using instanceof without something like canEqual violate the superclass contract. This is apparently widespread in code. Regardless what LSP means, i do believe that in an OO program, subclasses should fulfill the superclass contract.

What am I missing here?

To me this part:

Set<Point> hashSet1 = new java.util.HashSet<Point>();
hashSet1.add(p);
System.out.println(hashSet1.contains(cp)); // prints false

Set<Point> hashSet2 = new java.util.HashSet<Point>();
hashSet2.add(cp);
System.out.println(hashSet2.contains(p)); // prints true

seems correct.

In the first, we are asking "is there a red point at 1,2 in the collection?" and the answer is No - there is a point at 1,2 but we don't know if it's red or not (i.e. FALSE)

In the second we are asking "is there a point at 1,2 in the collection?" and the answer is yes - there's a point there (and it happens to be red" (i.e. TRUE)

So what's the problem with that implementation of hashCode?

> What am I missing here?
>
> In the first, we are asking "is there a red point at 1,2
> in the collection?" and the answer is No - there is a
> point at 1,2 but we don't know if it's red or not (i.e.
> FALSE)
>
> In the second we are asking "is there a point at 1,2 in
> the collection?" and the answer is yes - there's a point
> there (and it happens to be red" (i.e. TRUE)

You're asking two different questions, above. The first question is "Do we have two similarly coloured points with the same co-ordinates?" and the second question is "Do we have two points with the same co-ordinates, regardless of colour?". With two different questions, it's OK to get two different answers.

The equals(Object) method asks "Does this object and the other object have the same defining properties?" (i.e. it's similar to the first question, above). The javadoc for the equals(Object) method includes the following condition:

It is symmetric: for any non-null reference values x and y, x.equals(y) should return true if and only if y.equals(x) returns true.

When comparing a Point and a ColouredPoint the answer is always no because, whilst a ColouredPoint is a Point; a Point isn't a ColouredPoint. It doesn't matter which is the "this" object and which is the "other" object, one of them is missing a defining property that the other requires, so they can never be equal.

> > What exactly are we attempting to solve with this?
> Using
> > the most implementation-y form of inheritance to very
> > slightly reduce some minor code duplication? All so we
> can
> > have Point3D extend Point2D or Circle extend Ellipse?
> >
> The main point of the article is to try and spread the
> word more on the pitfalls of writing equals, and how to
> avoid them. Everything but the canEqual technique was
> already conventional wisdom, I think, but not necessarily
> widespread wisdom. So it doesn't hurt to remind folks.
>
> The point of canEqual is simply to show how to do
> something that I think was not conventional wisdom. The
> conventional wisdom, such as in Effective Java, is that
> this was not possible. I don't have a good use case off
> the top of my head, though.

I didn't mean to criticize the presentation itself, and I concur that further discussing this issue is a good thing in this instance.

However, I thought that the issue was that adding attributes to a concrete class is fundamentally at odds with the equivalence relation contracted by equals(). I'm surprised that there even is a workaround - I thought they were fundamentally incompatible!

In that sense, I definitely agree that the technique is interesting, and I didn't know about it, so I appreciate hearing about it.

> Somehow I didn't notice your comment about reducing code
> duplication. The problem wasn't code duplication, and I'm
> not sure which code you were thinking was being
> duplicated. The problem was that equals methods using
> instanceof without something like canEqual violate the
> superclass contract. This is apparently widespread in
> code. Regardless what LSP means, i do believe that in an
> OO program, subclasses should fulfill the superclass
> contract.

It seems to me that the only reason to subclass Point2D into Point3D would be to reduce duplication (private final <numeric-type> x, y; getX(), getY(), constructor; equals and hashCode implementation, maybe part of the toString implementation). I've long doubted the maxim that OO-maps-the-real-world, and I don't think Point3D is-a Point2D anyway (a 2D point in 3D is a line!), so I wouldn't model it that way.

Specifically, I was referring to this.x==that.x&&this.y==that.y: I don't think using a heavyweight technique like concrete implementation inheritance is called for simply to remove the duplication of those 15 tokens between Point2D and Point3D.

> In that sense, I definitely agree that the technique is
> interesting, and I didn't know about it, so I appreciate
> hearing about it.

I agree but I think it's a bit like learning that it's possible to have sustained powered flight for more than 3 seconds because someone did it for 4 seconds. The real problem that developers face that leads to all this navel staring is not resolved by this technique.

> It seems to me that the only reason to subclass Point2D
> into Point3D would be to reduce duplication (private final
> <numeric-type> x, y; getX(), getY(), constructor; equals
> and hashCode implementation, maybe part of the toString
> implementation). I've long doubted the maxim that
> OO-maps-the-real-world, and I don't think Point3D is-a
> Point2D anyway (a 2D point in 3D is a line!), so I
> wouldn't model it that way.

You have a point about 3D points not being 2D points but there are other examples that don't have this problem. The colored point is a better. Perhaps Person and Employee is a better example. Clearly an Employee is a Person. Is a Person object representing me equal to an Employee object representing me? It depends. If the Person object is a Customer, maybe not.

The real issue is that most Collections and Maps in the JDK depend on the equals (and sometimes hashcode) methods. Therefore the only way to use these standard collections in these situations is make equals work for all the different ways you want to compare things and that's often impossible (or infeasible) without violating the contract of equals.

The best solution is to make collections take a Equalator interface (similar to a Comparator). All of this nonsense goes away if you do that. Unfortunately the repeated cries for this feature from developers has been ignored by Sun for years.

Vincent wrote: "When comparing a Point and a ColouredPoint the answer is always no"

In that case, don't use instanceof, use

this.getClass().equals(that.getClass())


And you are done. But some disagree with your "always".

The original article said

"The idea is that as soon as a class redefines equals (and hashCode), it should also explicitly state that objects of this class are never equal to objects of some superclass that implement a different equality method"

To "solve" this issue, one trick I read somewhere (in earlier research of instanceof) is to declare your equals method final if you use instanceof.

So, if the the original class designer decrees that subclasses are only "a wee bit different" and can be equal, use final with instanceof. In general, the test will only involve fields of the superclass.

If the original designer is like Vincent and declares that "all subclasses are different", use getClass().

If the true answer is "it depends" or "the original designer was wrong", you'd need something like canEquals() or an Equalator.

Mental note: "Always avoid sweeping statements."

You're quite right right to point out that is is possible to make different classes (where one is a subclass of the other) equal without breaking the contract of the equals(Object) method and Bill has done an extremely excellent job of describing how it can be done.

Nevertheless, my conclusion from reading the article is that you should do everything possible to avoid needing such a solution on the grounds that it is unexpected that two object that are different things are the same. If only because the amount of code required to make it work, actually obscures what it is that you are trying to achieve. In addition, the mandatory requirement to implement a canEqual() method is, at best, fragile. I'm a great believer in the KISS principle.

I know it was just a toy example, but in this case the obvious solution would be for the Point object to implement a boolean isCoincident(Point other) method that just checks the X and Y co-ordinates for equality. Subclasses would then be automatically covered.

The original article was a real pleasure to read because of the way it revisted a core Java idiom and shed new light on it, in all the important areas:
- What we do do (but shouldn't),
- What we should do (but don't), and
- What we could do but...

> > It seems to me that the only reason to subclass Point2D
> > into Point3D would be to reduce duplication (private final
> > <numeric-type> x, y; getX(), getY(), constructor; equals
> > and hashCode implementation, maybe part of the toString
> > implementation). I've long doubted the maxim that
> > OO-maps-the-real-world, and I don't think Point3D is-a
> > Point2D anyway (a 2D point in 3D is a line!), so I
> > wouldn't model it that way.
>
> You have a point about 3D points not being 2D points but
> there are other examples that don't have this problem.
> The colored point is a better. Perhaps Person and
> Employee is a better example. Clearly an Employee is a
> Person. Is a Person object representing me equal to an
> Employee object representing me? It depends. If the
> Person object is a Customer, maybe not.
>
Employee/Person/Customer is a much better example, thank you.

In a domain model I design, there would be Employee and Customer classes because they represent concrete entities. But although I know in real life that employees and customers are people, I wouldn't model a Person superclass/type pre-implementation. The only way a Person class would exist would be if I had duplication to remove between Employee and Customer, and it wouldn't be at design time, but at implementation time.

> The real issue is that most Collections and Maps in the
> JDK depend on the equals (and sometimes hashcode) methods.
> Therefore the only way to use these standard collections
> in these situations is make equals work for all the
> different ways you want to compare things and that's often
> impossible (or infeasible) without violating the contract
> of equals.
>
I've solved this in the narrow way described above by Morgan Conrad, in a library for primary keys where the abstract class for numeric keys locked equals() to only match instances of the same subtype (eg, IntPK-equals-IntPK, not IntPK-equals-CharPK). But that's the narrowest way, not the general way of this article. I've just never come across a time when the nifty trick of using a canEquals() method would actually help me. Maybe I do too much business programming.

Have you (has anyone?) had success modeling a Person superclass/type (or moral equivalent) before implementation? My attempts failed early and often, due to issues like this one.

> The best solution is to make collections take a Equalator
> interface (similar to a Comparator). All of this nonsense
> goes away if you do that. Unfortunately the repeated
> cries for this feature from developers has been ignored by
> Sun for years.

Yes indeed. At least a single benevolent dictator can be compromised by taking him to a bar :-), it's much harder with corporate "benevolent dictators."

> The original article was a real pleasure to read because
> of the way it revisted a core Java idiom and shed new
> light on it, in all the important areas:
> - What we do do (but shouldn't),
> - What we should do (but don't), and
> - What we could do but...
>
Hear hear.

> I know it was just a toy example, but in this case the
> obvious solution would be for the Point object to
> implement a boolean isCoincident(Point other) method that
> just checks the X and Y co-ordinates for equality.
> Subclasses would then be automatically covered.

Absolutely and at the risk of beating a dead horse if we could use, say, a HashMap and tell it that we want isCoincident() to be the test for equality in one instance and the more strict equals in another, then we probably wouldn't be having this discussion. We'd use equals to be the canonical equivalence method. The canEquals() approach could still be a useful approach but it solves a pretty minor issue IMO.

hello from 2002!

http://www.ddj.com/java/184405053

Today only I was thinking, how to optimize writing hashCode and equals method.
In typical enterprise application, you lots of data beans which needs hashCode and equals.
Now right implementation of hashCode will always give unique number for each new object. So why not we check for equality of hashCode() == hashCode() in equals method.
So now, i have worried about only writing good hashCode method, instead of writing both.
-------------
public boolean equals(Object obj) {
if (obj == this) return true;
if (null == obj) return false;

return this.hashCode() == obj.hashCode();

}
--------------

> Now right implementation of hashCode will always give
> unique number for each new object. So why not we check for
> equality of hashCode() == hashCode() in equals method.

Strictly speaking, it's not possible to write a hashcode method that always produces a unique value. In a nutshell, the number of possible objects in a JVM instance could exceed the number of possible integer values.

More practically speaking, it's not really feasible to do this, especially considering that hashcode should be fast.

What methodology are you using to implement hashCode that you think will ensure uniqueness?