[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Field position attribute handling

From: Dalibor Topic
Subject: Re: [PATCH] Field position attribute handling
Date: Sun, 23 Nov 2003 21:40:05 +0100
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3) Gecko/20030312

Mark Wielaard wrote:

On Wed, 2003-11-19 at 02:14, Dalibor Topic 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

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:

I know. But that wasn't the point. If you change to code then you must
also update the documentation.

O.K., I just wans;t very sure about a non-ambigous wording. How about the attached patch?

2003-11-23  Dalibor Topic <address@hidden>

        * java/text/ (equals): Fixed comment.

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

It would be nice if we had something about the right way (tm) of writing
the standard clone(), toString(), hashCode() and equals() in the Hackers
Guide. (hint...)

I see. I'll try to come up with something based on what I've seen on the web and Bloch's book.

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.

Should yes, but not must. I was just wondering whether or not the
field_attribute really matters since that field and field_id indicate
the same property. Having cheap hashCode() methods is a good thing and
IMHO field_attribute does not make the hashCode() more specific. But if
you think otherwise then please do it as you have it now.

True. I'll write some test code, and may post another patch that removes it based on what I find.

dalibor topic
Index: java/text/
RCS file: /cvsroot/classpath/classpath/java/text/,v
retrieving revision 1.6
diff -u -r1.6
--- java/text/        19 Nov 2003 17:41:21 -0000      1.6
+++ java/text/        23 Nov 2003 20:34:53 -0000
@@ -168,7 +168,7 @@
    * <p>
    * <ul>
    * <li>The specified object is not <code>null</code>.
-   * <li>The specified object is an instance of <code>FieldPosition</code>.
+   * <li>The specified object has the same class as this object.
    * <li>The specified object has the same field identifier, field attribute 
    * and beginning and ending index as this object.
    * </ul>

reply via email to

[Prev in Thread] Current Thread [Next in Thread]