commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Stephen Colebourne" <scolebou...@joda.org>
Subject Re: JavaDoc of org.apache.commons.lang.builder.EqualsBuilder
Date Fri, 18 Oct 2002 23:06:02 GMT
[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 :-)

> 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.

> 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).

> 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.
>
> --- 8< ---
>
> public class A {
>    private int ai;
>    private static int as;
>    public boolean equals(Object other) {
>      if (other == null || this.getClass() != other.getClass()) { return
> false; }
>      A a = (A)other;
>      return this.ai = a.ai;
>    }
>    public int hashCode() {
>      return 0;
>    }
> }
>
> public class B extends A {
>    private int bi;
>    public boolean equals(Object other) {
>      if (!(super.equals(other)) { return false; }
>      B b = (B)other; // checked in superclass
>      return this.bi = b.bi;
>    }
> }
>
> --- 8< ---

Yes, superclasses is an area that could probably do with some more thought.


> Hope this is useful,
>
> Alex.

If you fancy sending in a documentation patch, we'd love to take a look ;-)

Stephen



--
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