<i18n dev> [14] RFR: 8222756: Plural support in CompactNumberFormat

[Roger Riggs]

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