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

Naoto Sato naoto at openjdk.org
Mon Apr 15 17:49:01 UTC 2024


On Wed, 10 Apr 2024 20:16:50 GMT, Justin Lu <jlu 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.

Comments for the `CompactNumberFormat`.

src/java.base/share/classes/java/text/CompactNumberFormat.java line 50:

> 48:  * <p>
> 49:  * {@code CompactNumberFormat} is a concrete subclass of {@code NumberFormat}
> 50:  * that formats a decimal number in a {@link Locale localized} compact form.

Same as in the NumberFormat

src/java.base/share/classes/java/text/CompactNumberFormat.java line 63:

> 61:  * <ul>
> 62:  * <li> Use {@link NumberFormat#getCompactNumberInstance()}
> 63:  * to obtain a {@code CompactNumberFormat} for the default locale.

Might add `Style.SHORT` as the default too

src/java.base/share/classes/java/text/CompactNumberFormat.java line 74:

> 72:  * <small>Note: It is recommended to use one of the NumberFormat factory methods
> 73:  * which is tailored to the conventions of the given locale to retrieve a
> 74:  * CompactNumberFormat.</small>

Not sure using the small font would make any significance here. Also, I'd make this a conditional sentence, i.e, if the user wants the standard formatting for a locale and a style, to avoid blind recommendation.

src/java.base/share/classes/java/text/CompactNumberFormat.java line 98:

> 96:  * The {@code compactPatterns} in {@link
> 97:  * CompactNumberFormat#CompactNumberFormat(String, DecimalFormatSymbols, String[])
> 98:  * CompactNumberFormat(decimalPattern, symbols, compactPatterns}} are represented

Nit: `}` -> `)`

src/java.base/share/classes/java/text/CompactNumberFormat.java line 103:

> 101:  *
> 102:  * <p><b>For those planning to only use the factory methods, the pattern syntax may
> 103:  * not be relevant. If this is the case, continue reading at the {@link ##formatting

Instead of adding reading suggestions like this in bold text, how about moving this `Compact Number Patterns` section to the very end of the class description, as it is less important for normal usage?

src/java.base/share/classes/java/text/CompactNumberFormat.java line 129:

> 127:  * minimum integer digits, and suffix. The negative subpattern
> 128:  * is optional, if absent, then the positive subpattern prefixed with the
> 129:  * minus sign ({@code '-'}) is used as the negative

The code point and its name should be preserved. "minus sign" can be ambiguous, as "MINUS SIGN" code point is actually `U+2212`

src/java.base/share/classes/java/text/CompactNumberFormat.java line 142:

> 140:  * {@linkplain DecimalFormat##special_pattern_character Special characters},
> 141:  * on the other hand, stand for other characters, strings, or classes of
> 142:  * characters. These characters must be quoted using single quotes ({@code '})

Same as above

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)

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?

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

PR Review: https://git.openjdk.org/jdk/pull/18731#pullrequestreview-1998701154
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1563335521
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1563346260
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1563343344
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1563354139
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1563353485
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1563355976
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1563356745
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1566208653
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1566221378


More information about the core-libs-dev mailing list