Re: [PATCH] Compliance of DecimalFormatSymbols

[Mark Wielaard]

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