Re: [PATCH] Field position attribute handling

[Dalibor Topic]

Hi Tom,

Tom Tromey wrote:
> > > > > > "Dalibor" == Dalibor Topic <address@hidden> writes:
>
>
> Dalibor> -    if (! (obj instanceof FieldPosition))
> Dalibor> +    if (this == obj)
> Dalibor> +      return true;
> Dalibor> +
> Dalibor> +    if (obj != null && (obj.getClass() != this.getClass()))
> Dalibor>        return false;
>
> I think this should read:
>
>   if (obj == null || obj.getClass() != this.getClass())
>     return false;

thanks for the comment. I agree that's more readable. I'll change that 
in another revision.

> I don't think short-circuiting the this==obj case is really worth the
> effort.  But I don't really care all that much.

A quick grep for equals in java.util.* collection classes shows that 
it's being used quite a bit there. for example, in order to find out 
whether an element is contained in a collection, on needs to find out 
whether there is an element that is equal to it in the collection. I 
believe that a short circuit here helps. I could run some tests and 
provide some numbers, if necessary.

> Dalibor> +    hash = 31 * hash + (null == field_attribute ? 0 : 
> field_attribute.hashCode());
>
> Does this line need to be wrapped?  I think it goes past column 79.

can't say, I need to teach emacs to count columns, I guess. But I'll 
wrap it anyway, to go for sure.

cheers,
dalibor topic