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

[Roger Riggs]

Looks good.

Thanks,  Roger

On 10/11/19 4:15 PM, naoto.sato at oracle.com wrote:
> Thanks, Roger. Modified readObject() accordingly:
>
> https://cr.openjdk.java.net/~naoto/8212749.8231984/webrev.01/
>
> Naoto
>
> On 10/11/19 10:41 AM, Roger Riggs wrote:
>> 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
>>>>>
>>