On Mon, 18 Nov 2024 14:21:29 GMT, Raffaello Giulietti <rgiulie...@openjdk.org>
wrote:
>> src/java.base/share/classes/java/math/BigDecimal.java line 2248:
>>
>>> 2246: }
>>> 2247: }
>>> 2248: } else { // mc.roundingMode == RoundingMode.UP ||
>>> mc.roundingMode == RoundingMode.CEILING
>>
>> It would be more robust to make all enum constant explicit, and reserve the
>> `else` to throw a `RuntimeException` of some sort, e.g., `MatchException`.
>> Should an additional enum constant be added to `RoundingMode` in the future,
>> this code will then throw instead of silently producing a wrong result.
>>
>> If possible at all, it would be preferable to use a `switch` rather than
>> `if`s.
>
> Also, please merge master into this.
While you are at it, it would be useful to add one test for each of the paths
in the new algorithm (including the exception cases) in the existing
[SquareRootTests](https://github.com/openjdk/jdk/blob/master/test/jdk/java/math/BigDecimal/SquareRootTests.java)
This helps to identify future regressions when the code is later modified for
any reason.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21301#discussion_r1847964055
