On Wed, 26 Mar 2025 20:31:07 GMT, Justin Lu <j...@openjdk.org> wrote:
> Please review this PR which clarifies the behavior for integer and fraction
> limits in NumberFormat and implementing classes. An associated CSR is filed.
>
> There have been a few bugs submitted which indicate a misconception that
> these limits impact parsing. The actual behavior is that these limits only
> affect formatting. The specification is vague regarding this, and can be
> explicitly updated to eliminate confusion. As the implementing classes are
> updated to use `inheritDoc`, some shuffling around in the method specs are
> included in this change as well.
>
> Alternatively I considered making this change as implementation specific to
> DecimalFormat and CompactNumberFormat only. (i.e. leave flexibility for other
> NumberFormat subclasses to define their own behavior on whether the limits
> affect parsing.) I am open to this option as well, but initially decided
> against it as
> 1) Unlike formatting, it seems like a rare use case that you would want to
> suppress the range of digits of accepted during parsing.
> `setParseIntegerOnly()` already provides functionality to toggle between
> integer and fraction parsing.
> 2) The limits affecting formatting only has been the long-standing behavior
> for all the subclasses of NumberFormat provided by the OpenJDK reference
> implementation.
Looks good
src/java.base/share/classes/java/text/DecimalFormat.java line 4047:
> 4045: * #setMaximumIntegerDigits(int)} or {@link #applyPattern(String)}.
> 4046: * See the {@link ##patterns Pattern Section} for comprehensive
> rules regarding
> 4047: * maximum integer digits in patterns.
This part only exists in this method and other getter methods do not have one.
Either removing this (as this is explained in the class description), or add it
to all other getters would be consistent ( the former is better IMO).
src/java.base/share/classes/java/text/NumberFormat.java line 938:
> 936: * also be set to the new value.
> 937: *
> 938: * @apiNote
I think this is `@implNote` as this is for subclass implemntors, not the users
of this API. (apply to other locations)
-------------
PR Review: https://git.openjdk.org/jdk/pull/24265#pullrequestreview-2718678916
PR Review Comment: https://git.openjdk.org/jdk/pull/24265#discussion_r2015035862
PR Review Comment: https://git.openjdk.org/jdk/pull/24265#discussion_r2015037836
