[14] RFR: 8222756: Plural support in CompactNumberFormat

[Joe Wang]

+1, looks good!


Best regards,
Joe


On 12/5/19 12:13 PM, Roger Riggs wrote:
> Hi Naoto,
>
> Thanks for the updates.
>
> Looks fine.
>
> Roger
>
>
> On 12/5/19 1:49 PM, naoto.sato at oracle.com wrote:
>> Thanks, Joe and Roger, for the reviews. Here is the updated webrev:
>>
>> http://cr.openjdk.java.net/~naoto/8222756/webrev.02/
>>
>> These are the changes since v.01:
>>
>> CompactNumberFormat.java:
>>
>> - Reflected the CSR changes (thank you, JoeD, for the quick 
>> turnaround!), and misc typos in the spec.
>>
>> - Added length limitation to the "pluralRules" argument in the new 
>> constructor. Throws IllegalArgumentException if the input is too long 
>> (>2,048 chars).
>>
>> - Added validation to plural rules, and throws 
>> IllegalArgumentException if the given rules' syntax is incorrect.
>>
>> - Consolidated the same pattern to get the affixes into a utility 
>> method (getAffix())
>>
>> - Directly find NaN and Infinity in the number string, parse the 
>> number with regex otherwise.
>>
>> TestEquality.java: Tidied the test array up.
>>
>> CLDRConverter.java: Added String::trim so that the generated 
>> PluralRules.java will not have extra spaces.
>>
>> TestPlurals.java: Added test cases for invalid cases, i.e., invalid 
>> syntax and length limit exceeding rules in the constructor.
>>
>> Not addressed:
>>
>> - ResourceBundleGenerator.java exists in the webrev as it does have a 
>> modification (only seen in "patch" link, as the mod is only the 
>> indentation)
>>
>> - Using "==" for double values: As Roger mentioned, it is in fact 
>> comparing integers (the only reason to use double is to deal with NaN 
>> and Infinity)
>>
>> - Possible performance improvement: Could be addressed later if it is 
>> regarded as an issue. But since the effective plural rules are 
>> simple/short enough, I would not expect it as a problem.
>>
>> Naoto
>>
>>
>>
>> On 11/26/19 1:35 PM, naoto.sato at oracle.com wrote:
>>> I modified CompactNumberFormat.java to simplify the syntax parsing:
>>>
>>> https://cr.openjdk.java.net/~naoto/8222756/webrev.01/
>>>
>>> Please review this webrev instead.
>>>
>>> Naoto
>>>
>>> On 11/25/19 1:16 PM, naoto.sato at oracle.com wrote:
>>>> Hello,
>>>>
>>>> CompactNumberFormat has been added since JDK 12 to support compact 
>>>> number formatting, such as 10_000 being formatted as "10K." [1] It 
>>>> works fine for English, however, not for other languages that take 
>>>> plural forms in formatted number prefixes/suffixes. In order to fix 
>>>> this, I filed the following CSR to extend the current 
>>>> CompactNumberFormat spec:
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8232633
>>>>
>>>> It basically accommodates the plural prefix/suffix forms into the 
>>>> existing compact patterns array, so that the existing compact 
>>>> number format works compatibly. The plural rules are solely based 
>>>> on the CLDR's plural language rules [2]
>>>>
>>>> Here is the implementation of the CSR:
>>>>
>>>> https://cr.openjdk.java.net/~naoto/8222756/webrev.00/
>>>>
>>>> Please review the CSR as well as its implementation.
>>>>
>>>> Naoto
>>>>
>>>>
>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8177552
>>>> [2] 
>>>> https://unicode.org/reports/tr35/tr35-numbers.html#Language_Plural_Rules 
>>>>
>