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

Naoto Sato naoto at openjdk.org
Mon Apr 15 18:46: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.

Here's the last part of the comments for `DecimalFormat`

src/java.base/share/classes/java/text/DecimalFormat.java line 58:

> 56: /**
> 57:  * {@code DecimalFormat} is a concrete subclass of
> 58:  * {@code NumberFormat} that formats decimal numbers in a {@link Locale localized} manner.

Same as the other two classes.

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.

src/java.base/share/classes/java/text/DecimalFormat.java line 116:

> 114:  * <p><b>For those planning to only use the factory methods, the pattern syntax may
> 115:  * not be relevant. If this is the case, continue reading at the {@link ##formatting Formatting and Parsing}
> 116:  * section.</b>

As in the `CompactNumberFormat`, I suggest moving the `DecimalFormat Pattern` section at the end.

src/java.base/share/classes/java/text/DecimalFormat.java line 168:

> 166:  * localized symbol.
> 167:  * When {@link #applyLocalizedPattern(String)} is called, the default symbols lose their
> 168:  * syntactical meaning, and vice versa with {@link #applyPattern(String)} with exception

Retain the original wording that explains localized and non-localized patterns at the beginning. Using the method example for the localized patterns might be a bit jump in the context.

src/java.base/share/classes/java/text/DecimalFormat.java line 255:

> 253:  * integer digits will be derived from the pattern. This derivation is detailed
> 254:  * in the {@link ##scientific_notation Scientific Notation} section. This behavior
> 255:  * is the typical end-user desire; {@link #setMaximumIntegerDigits(int)} can be

"This behavior ... desire" can be removed.

src/java.base/share/classes/java/text/DecimalFormat.java line 263:

> 261:  * subpattern has a prefix, numeric part, and suffix. The negative subpattern
> 262:  * is optional; if absent, then the positive subpattern prefixed with the
> 263:  * minus sign ({@code '-'}) is used as the

Keep the code point/name for hyphen-minus

src/java.base/share/classes/java/text/DecimalFormat.java line 398:

> 396:  * <h3>Special Values</h3>
> 397:  * <ul>
> 398:  * <li><p><b>Not a Number</b> ({@code NaN}) is successfully formatted as a string,

I'd not add `successfully` here.

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

PR Review: https://git.openjdk.org/jdk/pull/18731#pullrequestreview-2001855368
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1566248166
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1566260898
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1566263120
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1566272248
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1566279017
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1566280153
PR Review Comment: https://git.openjdk.org/jdk/pull/18731#discussion_r1566266870


More information about the core-libs-dev mailing list