+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.s...@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.s...@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.s...@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
