[Top][All Lists]

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

Re: [PATCH] NumberFormat

From: Dalibor Topic
Subject: Re: [PATCH] NumberFormat
Date: Sat, 22 Nov 2003 00:11:53 +0100
User-agent: Mozilla/5.0 (X11; U; Linux i586; en-US; rv:1.4) Gecko/20030624

Hi Guilhem,

Guilhem Lavaux wrote:

Here is just a patch to review about NumberFormat.Field before I check it in. It implements java.text.NumberFormat.Field it is quite straightforward using the documentation.

Looks good to me in general, I have small remarks before you check it in:

Index: java/text/
RCS file: /cvsroot/classpath/classpath/java/text/,v
retrieving revision 1.4
diff -u -r1.4
--- java/text/ 22 Jan 2002 22:27:01 -0000      1.4
+++ java/text/ 21 Nov 2003 19:52:44 -0000
@@ -79,6 +79,98 @@
+    private static final NumberFormat.Field[] allFields =
+    {
+    };

I don't know what GNU Classpath's policy on JavaDoc comments for private members [1] is, but as Michael Koch is cheering for checkStyle on #gcj I assume it will become the patch submission standard, and it pretty much wants to see comments on everything, if I recall my last CheckStyle usage attempts.

So, could you add a javadoc comment describing the field, please?

+    // For deserialization purpose
+    private Field()
+    {
+      super("");
+    }
Same as above.
Could you add a javadoc comment for the constructor, please?

+ + private Field(String s)
+    {
+      super(s);
+    }

This constructor is protected in the API spec for 1.4.2.

Again, a small comment would be nice.

+    protected Object readResolve() throws InvalidObjectException
+    {
+      String s = getName();
+      for (int i=0;i<allFields.length;i++)
+       if (s.equals(allFields[i].getName()))
+         return allFields[i];
+      throw new InvalidObjectException("no such NumberFormat field called " + 
+    }
+  }

Looks good to me. could you add another comment here?

Finally, the mean question: do we have mauve tests for those? If not, would you write some, please, so that gcj devs don't go ahead again and re-implement everything from scratch as they tend to do ;)? [2]

dalibor topic,

who is eager to write his own patch to the java/text/ (equals) method's comment as soon as he's finished playing virtual mjw ;)

[1] I devote this pun to ricky clarckson. Mark, how are his documentation patches coming along?

reply via email to

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