Subclasses and the equals method

[pek]

So I am currently reading Effective Java and in "Item 7: Obey the
general contract when overriding equals" Joshua Bloch talks about the
transitivity problem of equals when it comes to subclassing. To quote
from the book:
"There is simply no way to extend an instantiable class and add an
aspect while preserving the equals contract."

He gives an example of a Point object that has an x and y fields and
then extends it to a ColorPoint object with a Color field in it. He
then proves that there is no way of creating an equals method without
breaking the transitivity contract, i.e. this doesn't work correctly:

ColorPoint p1 = new ColorPoint(1, 2, Color.RED);
Point p2 = new Point(1, 2);
ColorPoint p3 = new ColorPoint(1, 2, Color.BLUE);

p1.equals(p2); // = true
p2.equals(p3); // = true
p1.equals(p3); // = false

In order to workaround this, his solution is that ColorPoint should
not extend Point, but rather contain an instance of point:

public class ColorPoint {
private Point point;
private Color color;

public ColorPoint(int x, int y, Color color) {
point = new Point(x, y);
this.color = color;
}

/**
* Returns the point-view of this color point.
*/
public Point asPoint() {
return point;
}
.....

While this works perfectly, it beats the benefits of subclassing an
object. So I'm thinking
"Why would you compare an object to it's superclass in the first
place?". Either you did it by accident, or you know you will have to
do this, so you change the equals of the superclass compromising it's
transitivity attribute.

But, I believe that there is more than this, so I my question is:

Is there an example which you would not extend a class in order to
compare it with it's superclass making sure it's transitive?

In no way I'm saying that he is wrong. I'm sure he is right, but he
doesn't provide a solid example that convinces me. His example with
Points didn't help me. I don't see why I would compare a Point with a
ColorPoint without knowing that I will have to breaking something. And
I can't think of a good example to perfectly understand his point.

Thank you,
Panagiotis

 

[pek]

Well, that just proves his point.  It really does not make sense to compare
instances of subclasses and their superclasses with each other for equality.
Any example that does so is going to seem flawed, at least in part because of
the issues Mr. Bloch mentions.  The notion of equality just doesn't carry
between instances of different classes very well at all, even when there's an
inheritance relation between them.

Note also his item to prefer composition to inheritance generally.

Yes, I agree. But he provides a workaround for it. Why? Where is this
workaround useful?

 

[Mark Space]

Points didn't help me. I don't see why I would compare a Point with a
ColorPoint without knowing that I will have to breaking something. And
I can't think of a good example to perfectly understand his point.

I think this is specifically where you're missing the crucial point.
Look at his examples with ColorPoint. OK, you can see something's going
on, but what?

I think here it is: in *Point* look at the equals method:

@Override
public boolean equals( Object o ) {
if( !(o instanceof Point) ) // <-- THE PROBLEM
return false;
Point p = (Point)o;
return p.x == x && p.y == y;
}

That's what he can't get around. When Point is defined, it doesn't know
what other points might subclass it. In Effective Java you know you
have a ColorPoint, but what about PixelPoint? MeshPoint? FanPoint?
(Names of OpenGL graphics structures in those last two.) Point only
knows about other Points and only compares x and y. That's all it can do.

That's what he's trying to maintain symmetry and transitivity with, and
he can't. That's why he's calling super.equals(), because he's showing
even doing that won't help. ColorPoint *is* a point, "instance of Point"
will be true, and the Point class will compare on x and y only, ignoring
the extra info. The only way around this is to compare ColorPoint on x
and y also, ignoring color, and that's clearly not right (which Bloch
mentions right at the start of that discussion).

Redo the examples in your head, paying close attention to the code above
when the Point class is involved. There's no way to extend the Point
class and still get it's equals method to work right, because when

Point p1 = ... anything
ColorPoint cp1 = ... anything

p1.equals( cp1 );

is invoked, p1 doesn't know to check for a subclass. It can't and can
only compare on x and y.


Inheritance only changes methodology or strategy, I guess. It doesn't
actually create a "subclass" in the everyday meaning of the word. A
ColorPoint is not really a Point, the two are completely different in
the way they work, due to the need to handle the extra field.


To me this says "if you define an equals method, make the class final."

Which is darn similar to what Scott Meyers and Bjarne Stroustrup have
been saying for a while: classes for designed inheritance should be
abstract. Concrete classes should be leaf classes (not inherited from).

 

[Joshua Cranmer]

To me this says "if you define an equals method, make the class final."

Or make the equals method final.

There is an inheritance scheme I'm using right now where the definition
of equality maps to something basic enough that the base class can
define it. Any other scheme where equality testing can be refactored to
"test that the two UUIDs are equal" can similarly be handled in this
fashion.

Although I should note that the lack of a final equals methods has
resulted in three separate instances of equals: a pointer-wise
comparison, uuid-based comparison, and a full property-wise-based (not
including uuid) comparison. Naturally, reflexive and transitive
properties have gone immediately out the window and I'm left with the
mess of trying to figure out which equals method to use for standardization.

 

[jolz]

There's no way to extend the Point

class and still get it's equals method to work right, because when

Point p1 = ... anything
ColorPoint cp1 = ... anything

p1.equals( cp1 );

is invoked, p1 doesn't know to check for a subclass.

It can be done:

public boolean equals( Object o ) {
if (o == null || !getClass().equals(o.getClass()))
return false;
....

 

[pek]

I think this is specifically where you're missing the crucial point.
Look at his examples with ColorPoint. OK, you can see something's going
on, but what?

I think here it is:  in *Point* look at the equals method:

@Override
public boolean equals( Object o ) {
   if( !(o instanceof Point) )  // <-- THE PROBLEM
     return false;
   Point p = (Point)o;
   return p.x == x && p.y == y;

}

That's what he can't get around.  When Point is defined, it doesn't know
what other points might subclass it.  In Effective Java you know you
have a ColorPoint, but what about PixelPoint? MeshPoint? FanPoint?
(Names of OpenGL graphics structures in those last two.)  Point only
knows about other Points and only compares x and y.  That's all it can do.

That's what he's trying to maintain symmetry and transitivity with, and
he can't.  That's why he's calling super.equals(), because he's showing
even doing that won't help. ColorPoint *is* a point, "instance of Point"
will be true, and the Point class will compare on x and y only, ignoring
the extra info.  The only way around this is to compare ColorPoint on x
and y also, ignoring color, and that's clearly not right (which Bloch
mentions right at the start of that discussion).

Redo the examples in your head, paying close attention to the code above
when the Point class is involved.  There's no way to extend the Point
class and still get it's equals method to work right, because when

Point p1 = ... anything
ColorPoint cp1 = ... anything

p1.equals( cp1 );

is invoked, p1 doesn't know to check for a subclass.  It can't and can
only compare on x and y.

Yes, I understanded that this will not work for subclassed object that
have an equal method. IIs there any example that you can think of
where you would like to compare a class with it's superclass? He
proves that this can't happen and you need a workaround (aspect), but
I can't think of why would you want that in the first place.

Inheritance only changes methodology or strategy, I guess. It doesn't
actually create a "subclass" in the everyday meaning of the word. A
ColorPoint is not really a Point, the two are completely different in
the way they work, due to the need to handle the extra field.

To me this says "if you define an equals method, make the class final."

Which is darn similar to what Scott Meyers and Bjarne Stroustrup have
been saying for a while:  classes for designed inheritance should be
abstract.  Concrete classes should be leaf classes (not inherited from)..

I must admit, I really liked this. Will try to follow it from now on.
Nice advise, thank you..

 

[pek]

Or make the equals method final.

There is an inheritance scheme I'm using right now where the definition
of equality maps to something basic enough that the base class can
define it. Any other scheme where equality testing can be refactored to
"test that the two UUIDs are equal" can similarly be handled in this
fashion.

Although I should note that the lack of a final equals methods has
resulted in three separate instances of equals: a pointer-wise
comparison, uuid-based comparison, and a full property-wise-based (not
including uuid) comparison. Naturally, reflexive and transitive
properties have gone immediately out the window and I'm left with the
mess of trying to figure out which equals method to use for standardization.

Although this would avoid comparing subclasses, it also doesn't permit
the subclass define an equals method. Well, at least the default
one. :S

 

[pek]

     Or use getClass() instead of instanceof:

        public boolean equals(Object obj) {
            if (obj == null
                || obj.getClass() != Point.class)
                return false;

            Point that = (Point)obj;
            return that.x == x && that.y == y;
        }

Hmmmmm... Nice.. This will work fine for a superclass.. What about
it's subclasses?

But anyway.. I do understand that you can't have transitive equality
in iheritance. But when do you want that? Does anybody has an example?

 

[Daniele Futtorovic]

It can be done:

public boolean equals( Object o ) {
if (o == null || !getClass().equals(o.getClass()))
return false;
....

However, that way you lose super.equals as a rump equality
implementation. Therefore it might be better to to it in two steps:

class X {
//yes, bad method name
protected boolean attributiveEquality(X x){
...
}

public boolean equals(Object o){
return o == this || o != null && getClass().equals(o.getClass()) &&
attributiveEquality( (X) o );
}
}

As for making equals final... I don't think it's a good idea. Only if
you strictly use composition over inheritance -- but even then. It's a
bit of throwing the baby out with the bath water.

 

[Mark Space]

Or use getClass() instead of instanceof:

public boolean equals(Object obj) {
if (obj == null
|| obj.getClass() != Point.class)
return false;

That's a nice idea. I wonder if it's truly correct for the vast
majority of cases where two "value" classes are compared for equality.

A better rule of thumb would be to implement an interface I think:

interface Point2D {
int getX();
int getY();
}

And then inject the comparison strategy somehow:

public class MyComparator<Point2D> {
public boolean myIsEqual( Point2D p1, Point2D p2 ) {
return p1.getX() == p2.getX() && p2.getY() && p2.getY();
}
}

But without careful design isEqual might just be the tip of the iceberg.

I want to emphasize that I think we are talking design patterns here and
rules of thumb, not specific instances of a design. What's tricky or
works for one design might not be well suit for many others. Making the
class final is a lot safer once things enter the maintenance stage.

 

[Mark Space]

I must admit, I really liked this. Will try to follow it from now on.
Nice advise, thank you..

See my idea above about defining an interface and having both classes
subclass from that. This is exactly the pattern Scott Myers recommends
in Effective C++ (which I just remembered and used.)

This is broken:

(---------)
( Class A )
(_________)
\
\
(---------)
( Class B )
(_________)


This is the correct way to switch this around


(---------)
( Abstract )
(_________)
/ \
/ \
(---------) (---------)
( Class A ) ( Class B )
(_________) (_________)


Now you don't have problems, and A and B both can be the same type
("Abstract") and not inherit from each other. A and B should be leaf
classes too. This is exactly what interfaces do for you in Java. I
can't think off the top of my head where too classes inherit and are
directly comparable. Something in collections might be a worthy
candidate, where you subclass a HashMap for example, to add some
function. I think Block is showing that this has to be done with great
care...

 

[Mark Space]

"Bloch", not "Block".

"Bloch" is a venerable and worthy clan.

Whoops. I'm pleading the spell-checker defense. (Though I probably
mistyped it and didn't notice...)

 

[Lew]

Whoops. I'm pleading the spell-checker defense. =A0(Though I probably
mistyped it and didn't notice...)

I'm jut a tad incomplete over that lengthy spelling issue.

--
Lew Bloch


- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
"We probably have given this president more flexibility, more
latitude, more range, unquestioned, than any president since Franklin
Roosevelt -- probably too much. The Congress, in my opinion, really
abrogated much of its responsibility."

--- Sen. Chuck Hagel (R-Neb.),
a senior member of the Foreign Relations Committee

 

[Lew]

Whoops. I'm pleading the spell-checker defense.  (Though I probably
mistyped it and didn't notice...)

I'm jut a tad sensitive over that particular spelling issue.

 

[pek]

     Or use getClass() instead of instanceof:

        public boolean equals(Object obj) {
            if (obj == null
                || obj.getClass() != Point.class)
                return false;

            Point that = (Point)obj;
            return that.x == x && that.y == y;
        }

Turns out that using .getClass isn't a good workaround. Bloch says:
"You may hear it said that you can extend an instantiable class and
add a value
component while preserving the equals contract by using a getClass
test in
place of the instanceof test in the equals method"
*snip*
"This has the effect of equating objects only if they have the same
implementation
class. While this may not seem so bad, the consequences are
unacceptable."

he proves his point using hashmaps..

 

[EJP]

Bloch says:
"You may hear it said that you can extend an instantiable class and
add a value component while preserving the equals contract by using a getClass
test in place of the instanceof test in the equals method"
*snip*
"This has the effect of equating objects only if they have the same
implementation
class. While this may not seem so bad, the consequences are
unacceptable."

he proves his point using hashmaps..

Whatever Bloch may say, the case of RMI client socket factories is an
important counter-example. javax.rmi.ssl.SslRMISocketFactory does
precisely this, and it is advocated all over the RMI mailings lists and
various books, including my own.