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

[Justin Lu]

On Tue, 16 Apr 2024 21:14:28 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
>
> src/java.base/share/classes/java/text/CompactNumberFormat.java line 78:
> 
>> 76:  * installed. Thus, to use an instance method defined by {@code CompactNumberFormat},
>> 77:  * the {@code NumberFormat} returned by the factory method should first be type
>> 78:  * checked before cast to {@code CompactNumberFormat}. If the installed locale-sensitive
> 
> Since `CompactNumberFormat` does not provide its own instance methods (i.e., instance methods are exactly the same as the parent NumberFormat), "instance methods defined by CompactNF" does not make much sense, which is different for `DecimalFormat`.

`CompactNumberFormat` does define its own instance methods such as `setParseBigDecimal`, `setGroupingSize`, etc, so I think that this wording should still be included.

> src/java.base/share/classes/java/text/NumberFormat.java line 91:
> 
>> 89:  * for example, {@link #getIntegerInstance(Locale)}. If the installed locale-sensitive
>> 90:  * service implementation does not support the given {@code Locale}, {@link Locale#ROOT}
>> 91:  * will be used as the fallback {@code Locale}.
> 
> The fallback traverses the parent locales, and checks if it is supported or not. So it may or may not reach Locale.ROOT. How about "it looks up the parent locale chain and uses the one that is supported" or something along the lines?

Thanks for the clarification, updated the wording.

> src/java.base/share/classes/java/text/NumberFormat.java line 235:
> 
>> 233:  * @see          ChoiceFormat
>> 234:  * @see          CompactNumberFormat
>> 235:  * @see          Locale
> 
> Could be removed as the link is now gone

As this is a localized class, and the class description mentions `Locale` in various places, I feel like `Locale` should still be included as a `see` tag, (even though I removed the locale link). I am fine either way, what do you think?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1568004434
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1568004370
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1568004906