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

[naoto.sato at oracle.com]

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
>>>>
>