[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Compliance of DecimalFormatSymbols
From: |
Mark Wielaard |
Subject: |
Re: [PATCH] Compliance of DecimalFormatSymbols |
Date: |
Sat, 22 Nov 2003 08:16:43 +0100 |
Hi,
On Sat, 2003-11-22 at 00:39, Dalibor Topic wrote:
> > Here is an adaptation of the former patch concerning java/text.
> > This one is small and concerns only java/text/DecimalFormatSymbols.
> > According to the official specification, if the serial version is greater
> > than 2, you have to read the locale field. This is what is done here.
Thanks. Below are some little (syntactic) nitpicks.
> @@ -580,13 +581,20 @@
> /**
> * @serial This value represents the type of object being de-serialized.
> * 0 indicates a pre-Java 1.1.6 version, 1 indicates 1.1.6 or later.
> - */
> - private int serialVersionOnStream = 1;
> + * 0 indicates a pre-Java 1.1.6 version, 1 indicates 1.1.6 or later,
> + * 2 indicates 1.4 or later
> + */
> + private int serialVersionOnStream = 2;
Indentation looks strange (hopefully jalopy will catch that for us in the
future).
And comments should end with a dot.
>
> + /**
> + * @serial the locale of these currency symbols.
> + */
> + private Locale locale;
Make that comment start with a capital (The local...) so it looks like a real
sentence.
> {
> monetarySeparator = decimalSeparator;
> exponential = 'E';
> - serialVersionOnStream = 1;
> }
> + if (serialVersionOnStream < 2)
> + {
> + locale = Locale.getDefault();
> + }
> + serialVersionOnStream = 2;
> }
> }
You don't need brackets when the block is just one line:
if (serialVersionOnStream < 2)
locale = Locale.getDefault();
serialVersionOnStream = 2;
> Looks good to me beside just a small point. Could you write some mauve
> tests to go with it, please?
Already made a very simple one.
gnu/testlet/java/text/DecimalFormatSymbols/serial.java
It can certainly be extended to test for more things.
> > 2003-11-21 Guilhem Lavaux <address@hidden>
> >
> > * java/text/DecimalFormatSymbols.java (locale): New field.
> > (serialVersionOnStream): Upgraded to number 2
> > (readObject): updated to assign locale if it wasn't by the
> > serializer.
>
> Lacks one entry:
> (DecimalFormatSymbols (Locale)): Set locale.
>
> Other than that, looks fine to me. I'm waiting for the ChangeLog police,
> though ;)
At your service!
- ChangeLog entries should start with:
date <two spaces> Name <two spaces> email.
- Just <one space> between filename and (field/method)
- Entries are full sentences with starting capital and ending dot.
2003-11-21 Guilhem Lavaux <address@hidden>
* java/text/DecimalFormatSymbols.java (locale): New field.
(DecimalFormatSymbols (Locale)): Set locale.
(serialVersionOnStream): Upgraded to number 2.
(readObject): Assign locale if it wasn't by the serializer.
Guilhem, please feel free to try out your new cvs commit powers with
these changes.
Cheers,
Mark
signature.asc
Description: This is a digitally signed message part