RFR: 8314169: Combine related RoundingMode logic in j.text.DigitList

[Naoto Sato]

On Fri, 11 Aug 2023 18:27:47 GMT, Justin Lu <jlu at openjdk.org> wrote:

> Please review this PR which is a broad clean up of the DigitList class (used by Format classes in j.text).
> 
> This PR is intended to be a portion of a bigger change (split up to make reviewing easier). 
> 
> The main change combines related Rounding Mode logic in `shouldRoundUp()` - (_CEILING/FLOOR_, _HALF_UP/DOWN/EVEN_)
> 
> Other changes include
> - Certain for loops can be replaced with cleaner syntax (E.g. for(;;), empty for loops)
> - Introduce overloaded `round(int)` - For use by Integer representations of DigitList
> - Introduce `non0AfterIndex(int)` - To reduce code duplication

src/java.base/share/classes/java/text/DigitList.java line 123:

> 121:      * from the given index until the end.
> 122:      */
> 123:     private boolean non0AfterIndex(int index) {

I'd prefer spelling out `0` to `Zero`.

src/java.base/share/classes/java/text/DigitList.java line 409:

> 407:      * Upon return, count will be less than or equal to maximumDigits.
> 408:      */
> 409:     private void round(int maximumDigits) {

If the method is only used for `Long` and `BigInteger`, probably use the specific name instead of explaining it in the description and overloading would be more clear to me.

src/java.base/share/classes/java/text/DigitList.java line 521:

> 519:                 if (non0AfterIndex(maximumDigits)) {
> 520:                     return (isNegative && roundingMode == RoundingMode.FLOOR)
> 521:                             || (!isNegative && roundingMode == RoundingMode.CEILING);

`roundingMode` is checked against `FLOOR`/`CEILING` twice. I don't see the need of bundling them up.

src/java.base/share/classes/java/text/DigitList.java line 526:

> 524:             case HALF_UP:
> 525:             case HALF_DOWN:
> 526:                 case HALF_EVEN:

Fix the indentation

src/java.base/share/classes/java/text/DigitList.java line 543:

> 541:                                                 && (maximumDigits > 0) && (digits[maximumDigits - 1] % 2 != 0));
> 542:                                 // Not already rounded, and not exact, round up
> 543:                             } else {

Are you sure this logic is equivalent to the previous one? Previously, `HALF_UP/DOWN` checks `valueExactAsDecimal` before `alreadyRounded`, but the new one checks `alreadyRounded` first.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15252#discussion_r1291822984
PR Review Comment: https://git.openjdk.org/jdk/pull/15252#discussion_r1291829499
PR Review Comment: https://git.openjdk.org/jdk/pull/15252#discussion_r1291833892
PR Review Comment: https://git.openjdk.org/jdk/pull/15252#discussion_r1291834906
PR Review Comment: https://git.openjdk.org/jdk/pull/15252#discussion_r1291843188