Re: [PATCH] Field position attribute handling

[Dalibor Topic]

Hi Mark,

Mark Wielaard wrote:

> The documentation and code don't match up now.
> My Class Library book seems to indicate that it actually is instanceof,
> thought in most cases your code comparing the actual Class seems more
> correct.

It's the recommended way of doing equals() if you want to avoid symmetry
bugs. See PRAXIS 12 in Peter Haggar's Practical Java, or §5.1, p 119 in
Java 2 performance and idion guide by craig lahrmann & rhett guthrie, or
Effective Java by Joshua Bloch. Using instanceof is a bad idea because
it allows for symmetry bugs:

B extends A. With instanceof in equals, we can have A.equals(B) but 
!B.equlas(A) since A is not an instance of B. Ugly, and very broken. 
There are a few places, where the API demands this (Maps), but they are 
all well documented, AFAIK. If it's not documented, it's better to use 
the symmetry safe form, in my opinion.

> > +  /**
> > +   * This method returns a hash value for this object
> > +   * 
> > +   * @return A hash value for this object.
> > +   */
> > +  public int hashCode ()
> > +  {
> > +    int hash = 7;
> > +
> > +    hash = 31 * hash + field_id;
> > +    hash = 31 * hash + begin;
> > +    hash = 31 * hash + end;
> > +    hash = 31 * hash + (null == field_attribute ? 0 : 
> > field_attribute.hashCode());
> > +
> > +    return hash;
> > +  }
>
>
> Just curious, why this particular hash function?

7 is the largest prime, that's still an iconst, iconst_7, afaik. That 
makes it take less bytecode.

The rest follows recommendations from Joshua Bloch on writing good 
hashCode methods.

> I couldn't find one specified, so I guess any good one will do.
> Don't know if you need to include the field_attribute.hashCode() since
> it will probably depend on the field_id in almost all cases.

If two objects are not equal, they should have different hashCodes. 
field_attribute is checked in the equality test, so it should play a 
role in hashCode calculation as well.

cheers,
dalibor topic