commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Steve Downey <steve.dow...@netfolio.com>
Subject Re: JavaDoc of org.apache.commons.lang.builder.EqualsBuilder
Date Sat, 19 Oct 2002 15:38:24 GMT
On Friday 18 October 2002 07:06 pm, Stephen Colebourne wrote:
> [Alex, I have replied to commons-dev mailing list as the most appropriate
> place for the discussion]
>
> Inline...
> ----- Original Message -----
> From: "Alex Blewitt" <Alex.Blewitt@ioshq.com>
>
> > I've just read the JavaDoc for EqualsBuilder V 1.0 on the Apahce
> > website, and have a few comments which I think you may like to take
> > into account:
> >
> > o You shouldn't use 'instanceof' in the test for equality of type. You
> > should instead use this.getClass() == other.getClass(). The simple
> > reason for this is the equals method is meant to be reflexive (i.e.
> > a.equals(b) == b.equals(a)) and using 'instanceof' it is possible to
> > break that contract. For example, classes A and B (extends A), then
> > a.equals(b) will return true (even if there are attributes of 'b' that
> > are added or different) and b.equals(a) will never be true, even if
> > there are no attributes of 'b' added. Note that this also works for
> > superclass equality; it is safe to use 'super.equals(other)' if there
> > are other tests that need to be done. Note that the rules, laid out by
> > Joshua Bloch, are generally regarded as false since it breaks the
> > assumptions of the equality method and have widely been ridiculed.
> > [However, note that you need to test for 'other==null' since null
> > instanceof X always returns false.]
>
> I think I agree about the instanceof check in the code. It probably should
> be class equality.
>
> I am unclear as to what you find exactly wrong with Josh Bloch's book. I
> have never heard of it being ridiculed, but then maybe I'm not in the right
> circles :-)
>

Ridicule is probably a bit strong. But what does turn out to be the case is 
that inheritance and equals don't mix. Even if you fix the reflexive problem, 
by slicing, that will break transitivity.  super1 = sub1, super1 = sub2, but 
sub1 != sub2. 

Here's the best discussion on the subject I could find: 
http://www.cuj.com/java/articles/a19.htm?topic=java

It's an article by Angelika Langer and Klaus Kreft, reviewing the state of the 
art in equals(). One thing that they uncover is that if the base class uses 
instanceof, so should the sub class. And, the practice in the standard 
library is to use instanceof. 


> > o You also comment that any field used in equality testing must be used
> > in hashcode, and vice versa. The reverse is not true. You can have a
> > hashCode method that returns a constant '0' (thereby not using any of
> > the fields) with an implementation of equals that works for any (or
> > all) fields.
>
> Correct, the javadoc is probably over harsh.

Although it's probably good practice. A bad hash code really messes up a 
hashtable. Instead of the O(1) you were expecting, you end up with O(n), 
since it collapses to an expensive list.

>
> > o You don't point out that static and non-transient  fields should not
> > be used in the implementation of equals, which is required as per the
> > spec.
>
> I assume you mean transient. I reckon they don't harm if they are checked
> (static final and transient, static would harm).

Static shouldn't harm either. It just shouldn't be necessary.

>
> > o You don't give an example of how to integrate with a superclass. In
> > general, for classes that have an implementation of an equals method in
> > a (non-Object) superclass, you should also have a first line test
> > 'super.equals(other)' as well.
> >

The best thing to point out is that value-type heirarchies are to be avoided. 
They have deep conceptual problems. Like the problems with equals. 
Composition, rather than inheritance, seems to be the right thing to do.

However, if you are stuck with it, then knowing what you are getting yourself 
into is probably a good idea.



--
To unsubscribe, e-mail:   <mailto:commons-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:commons-dev-help@jakarta.apache.org>


Mime
View raw message