RFR: 8329222: java.text.NumberFormat (and subclasses) spec updates [v2]

[Justin Lu]

On Mon, 15 Apr 2024 18:43:29 GMT, Naoto Sato <naoto at openjdk.org> wrote:

>> Justin Lu has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains five commits:
>> 
>>  - merge master and add setStrict() to nFmt class spec
>>  - implement suggestions from dFmt review
>>  - implement suggestions from cnFmt review
>>  - implement suggestions from nFmt review
>>  - init
>
> Here's the last part of the comments for `DecimalFormat`

@naotoj The latest commits should implement your batch of suggestions. I have also re-built the docs and specdiff with these changes.

> src/java.base/share/classes/java/text/CompactNumberFormat.java line 154:
> 
>> 152:  * </sup>) in the US locale can be specified as the SimplePattern: {@code "0 Million"}, for the
>> 153:  * German locale it can be specified as the PluralPattern:
>> 154:  * {@code "{one:0' 'Million other:0' 'Millionen}"}.
> 
> I like the German example, however, I think the original explanation for the pattern should be preserved (wording might be better though)

I originally removed it because the syntax already covered proper usage. But I can see how it can be confusing at first to understand. I re-inserted the original wording, but modified it IMO to be easier to understand.

> src/java.base/share/classes/java/text/CompactNumberFormat.java line 207:
> 
>> 205:  * {@link java.math.RoundingMode} for formatting.  By default, it uses
>> 206:  * {@link java.math.RoundingMode#HALF_EVEN RoundingMode.HALF_EVEN}.
>> 207:  *
> 
> Any reason for this part to move before the `Parsing` section?

I moved it before `Parsing` as it is related to formatting, so its better under the `Formatting` section.

All the other CompactNumberFormat comments should be implemented as well.

> src/java.base/share/classes/java/text/DecimalFormat.java line 93:
> 
>> 91:  * for example, {@link #getIntegerInstance(Locale)}. {@link
>> 92:  * NumberFormat#getAvailableLocales()} can be used to determine if the desired
>> 93:  * locale is supported.
> 
> I don't think this section is helpful, or rather, may mislead the users. As the original text explains, factory methods in `NumberFormat` do not guarantee to return `DecimalFormat` instances by design.

OK, that's a good point. In all 3 of these classes, I adjusted the wording to recommend using the factory methods for a locale's standard formatting. However, I made it apparent that the resultant Object can be any subclass of NumberFormat, and that if users want to cast, (so that they have access to the subclass instance methods), they should do appropriate type checking first.

All the other DecimalFormat comments you left should be implemented as well.

> src/java.base/share/classes/java/text/NumberFormat.java line 118:
> 
>> 116:  *
>> 117:  * <h2>Customizing NumberFormat</h2>
>> 118:  * {@code NumberFormat} provides API to customize formatting and parsing behavior,
> 
> Could make this PR a dependent PR of the leniency PR, and add `setStrict()` here as well.

The leniency PR was integrated, I directly added setStrict() to this PR

-------------

PR Comment: https://git.openjdk.org/jdk/pull/18731#issuecomment-2059552391
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1567700054
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1567696852
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1567696727
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1567698329