<i18n dev> RFR: 8329222: java.text.NumberFormat (and subclasses) spec updates

[Justin Lu]

On Fri, 12 Apr 2024 20:58:07 GMT, Naoto Sato <naoto at openjdk.org> wrote:

>> Please review this PR which is a large spec reformatting for _java.text.NumberFormat_ and related subclasses, specifically DecimalFormat and CompactNumberFormat.
>> 
>> The motivation for this change was the difficulty of readability for these classes. This PR consists of changes such as better separating the text into sections with headers, advising to skip the pattern related sections if not needed, and general wording improvements.
>> 
>> To help with reviewing I have attached a [specdiff](https://cr.openjdk.org/~jlu/JDK-8329222/java.base/java/text/package-summary.html) as well as the built docs: [nFmt](https://cr.openjdk.org/~jlu/api/java.base/java/text/NumberFormat.html), [dFmt](https://cr.openjdk.org/~jlu/api/java.base/java/text/DecimalFormat.html), and [cnFmt](https://cr.openjdk.org/~jlu/api/java.base/java/text/CompactNumberFormat.html);
>> 
>> I will update the CSR once the wording is finalized.
>
> Hi Justin, I went through the `NumberFormat` part. More to follow.

Hi @naotoj , thanks for the detailed review. Working on the suggestions; left a few responses as well.

> src/java.base/share/classes/java/text/NumberFormat.java line 61:
> 
>> 59:  * {@code NumberFormat} is the abstract base class for all number
>> 60:  * formats. This class provides the interface for formatting and parsing
>> 61:  * numbers in a {@link Locale localized} manner. This enables code that can be completely
> 
> Linking "localized" to the `Locale` class seems a bit odd to me. A user who clicks the link may be stranded.

Makes sense, I'll remove the link.

The intent was to give a reader unfamiliar with i18n a little more background. But the next sentence expands on locale in that sense so perhaps that's enough.

> src/java.base/share/classes/java/text/NumberFormat.java line 66:
> 
>> 64:  * digits used. For example, this class could be used within an application to
>> 65:  * produce a number in a currency format according to conventions of the locale
>> 66:  * a user resides in.
> 
> "a user resides in" may not convey any additional value. The user doesn't have to be in that locale.

Fair enough, I don't want to give any misconceptions.

The point was to give a general use case example of an app allowing format(s) tailored to the locale of the user. As I imagine this would be the most common case, rather than getting a locale separate than that of the user. I'll adjust the example.

> src/java.base/share/classes/java/text/NumberFormat.java line 91:
> 
>> 89:  * locale is supported by the installed locale-sensitive service implementation,
>> 90:  * either use {@link #getAvailableLocales()} or ensure a factory method call is enclosed
>> 91:  * within a try block.
> 
> Not sure this is correct. Even if a locale is not "supported" by a locale provider, it won't throw an exception, but fallback to, say ROOT, locale.

Good to know, I didn't realize this was the case. This is inherited from the original wording: `"This will work for the vast majority of locales; just remember to put it in a try block in case you encounter an unusual one."`

I'll replace it with the actual behavior, that Locale.ROOT is a fallback, if the particular locale is not supported.

> src/java.base/share/classes/java/text/NumberFormat.java line 135:
> 
>> 133:  * {@code CompactNumberFormat} depending on the factory method used. For example,
>> 134:  * cast to {@code DecimalFormat} to call {@link DecimalFormat#setGroupingSize(int)}
>> 135:  * to change the desired digits between grouping separators.
> 
> Since there is a possibility that SPIs can return the NumberFormat instances, not sure recommending casting is the right thing. If you want to add this, at least suggest the type check beforehand.

This is also inherited from the original wording: `"If you want even more control over the format or parsing, or want to give your users more control, you can try casting the NumberFormat you get from the factory methods to a DecimalFormat or CompactNumberFormat depending on the factory method used."`

True that an SPI can return some other NumberFormat instance, but I think that we still need to emphasize casting to the JDK DecimalFormat or CompactNumberFormat, otherwise users have no way to access the subclass instance methods, unless they directly construct the subclass which we don't recommend.

I'll adjust the wording here (and include type checking)

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

PR Comment: https://git.openjdk.org/jdk/pull/18731#issuecomment-2052601485
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1563246011
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1563245987
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1563246086
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1563246207