<i18n dev> [14] RFR 8212749: DecimalFormat.setGroupingSize(int) allows setting negative grouping size, 8231984: Clarify semantics of DecimalFormat.getGroupingSize(0)

[Roger Riggs]

Hi Naoto,

The javadoc/spec comments look fine.

Code comments at DecimalFormat:4035 give some latitute for the value to be
out of range and since getGroupingSize returns the groupingSize byte
it would be cleaner if the value was always in the valid range
regardless of the isGroupingUsed boolean.

Can there be code in the readObject method to correct out of range
values, perhaps to the default = 3?

Thanks, Roger



On 10/9/19 1:47 PM, naoto.sato at oracle.com wrote:
> I revised the fix, incorporating the clarification of the value zero 
> as the grouping size, which has separate JIRA issue and CSR as follows:
>
> https://bugs.openjdk.java.net/browse/JDK-8231984
> https://bugs.openjdk.java.net/browse/JDK-8232012
>
> The merged changeset is as follows:
>
> http://cr.openjdk.java.net/~naoto/8212749.8231984/webrev.00/
>
> Please review.
>
> Naoto
>
> On 10/8/19 8:59 AM, naoto.sato at oracle.com wrote:
>> Hi Roger,
>>
>> Thank you for the review. In fact, Joe commented about the validity 
>> of zero on the CSR, so I will need to modify the method description 
>> such as:
>>
>> diff -r 9576895d0f9a 
>> src/java.base/share/classes/java/text/DecimalFormat.java
>> --- a/src/java.base/share/classes/java/text/DecimalFormat.java
>> +++ b/src/java.base/share/classes/java/text/DecimalFormat.java
>> @@ -2770,10 +2770,13 @@
>>       /**
>>        * Set the grouping size. Grouping size is the number of digits 
>> between
>>        * grouping separators in the integer portion of a number.  For 
>> example,
>> -     * in the number "123,456.78", the grouping size is 3.
>> -     * <br>
>> +     * in the number "123,456.78", the grouping size is 3. Grouping 
>> size of
>> +     * zero designates that grouping is not used, which provides the 
>> same
>> +     * formatting as if calling {@link #setGroupingUsed(boolean)
>> +     * setGroupingUsed(false)}.
>> +     * <p>
>>        * The value passed in is converted to a byte, which may lose 
>> information.
>> -     * Invalid value, i.e., negative or greater than
>> +     * Values that are negative or greater than
>>        * {@link java.lang.Byte#MAX_VALUE Byte.MAX_VALUE}, will throw an
>>        * {@code IllegalArgumentException}.
>>        *
>>
>> I will file a follow-on CSR and merge changesets.
>>
>> Naoto
>>
>> On 10/8/19 6:59 AM, Roger Riggs wrote:
>>> Hi Naoto,
>>>
>>> DecimalFormat.java: 2776:  "Invalid value, i.e.," -> "Values that are".
>>>
>>> Otherwise looks fine. No need for another webrev.
>>>
>>> Thanks, Roger
>>>
>>>
>>>
>>>
>>> On 10/4/19 6:54 PM, naoto.sato at oracle.com wrote:
>>>> Hello,
>>>>
>>>> Please review the fix to the following issue:
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8212749
>>>>
>>>> The proposed CSR and changeset are located at:
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8231851
>>>> https://cr.openjdk.java.net/~naoto/8212749/webrev.00/
>>>>
>>>> Naoto
>>>