|
From: | Dalibor Topic |
Subject: | Re: [PATCH] Field position attribute handling |
Date: | Wed, 19 Nov 2003 02:38:48 +0100 |
User-agent: | Mozilla/5.0 (X11; U; Linux i586; en-US; rv:1.4) Gecko/20030624 |
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
[Prev in Thread] | Current Thread | [Next in Thread] |