[14] RFR: 8222756: Plural support in CompactNumberFormat
Roger Riggs
Roger.Riggs at oracle.com
Wed Dec 4 20:16:40 UTC 2019
Hi Naoto,
Looks very good, and a few comments...
CompactNumberFormat.java:
144: "each singular and plural patterns" -> "each singular and plural
pattern"
148: "Should the pattern include" -> "If the pattern includes"
336: Add a "." at the end of the sentence
407/417: If the plural rules is not syntatically correct, how is the
error reported? IllegalArgumentException?
587-588: Can be simplified a bit:
The pattern appears multiple times and could be a utility method.
private String getPrefix(boolean isNegative, int compactDataIndex,
int iPart) {
return (isNegative ? negativePrefixPatterns :
positivePrefixPatterns)
.get(compactDataIndex).get(iPart);
}
1667: Pattern DIGITS; is there a pattern syntax to surround the inf and
nan parts
to ensure they are treated as literals?
Can Pattern.compile throw? The parse(text,pos) method does not declare
IllegalArgumentException.
It might be clearer to check for inf/nan outside the digits parse pattern.
2378: Using == on doubles seems like a risky choice but may be ok given
that the input is integers, not floating point.
2350-2353 Looks ok like a lot of pattern matching on every parse.
If performance turns out to be an issue, it should be possible to parse
and generate a tree of lambdas with bindings to the literal values.
The lambda expressions could be saved and re-used.
TestEquality.java:
51: please break the line into reasonable line lengths.
Generated PluralRules.java: Each plural entry has an extra space at the
end of the string;
CLDRConverter.java:
1215: Please trim strings to avoid looking like they are significant.
Regards, Roger
ResourceBundleGenerator.java does not have any changes and may not need
to be in the webrev.
On 11/26/19 4: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
More information about the core-libs-dev
mailing list