Guilhem Lavaux wrote:
Hi,
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:
[snip]Index: java/text/NumberFormat.java =================================================================== RCS file: /cvsroot/classpath/classpath/java/text/NumberFormat.java,v retrieving revision 1.4 diff -u -r1.4 NumberFormat.java --- java/text/NumberFormat.java 22 Jan 2002 22:27:01 -0000 1.4 +++ java/text/NumberFormat.java 21 Nov 2003 19:52:44 -0000 @@ -79,6 +79,98 @@
+ private static final NumberFormat.Field[] allFields =
+ {
+ INTEGER, FRACTION, EXPONENT, DECIMAL_SEPARATOR, SIGN,
+ GROUPING_SEPARATOR, EXPONENT_SYMBOL, PERCENT,
+ PERMILLE, CURRENCY, EXPONENT_SIGN
+ };
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 " + s);
+ }
+ }
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]
cheers, dalibor topic,
who is eager to write his own patch to the java/text/FieldPosition.java (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?
_______________________________________________ Classpath mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/classpath
