On Fri, 27 Aug 2021 18:53:23 GMT, Raffaello Giulietti
<[email protected]> wrote:
> Please review this PR to add officially endorsed `ceilDiv()` and `ceilMod()`
> methods do `j.l.[Strict]Math`.
>
> Beside adding fresh tests to `test/jdk/java/lang/Math/DivModTests.java`, this
> PR also corrects small typos in it and exercises tests that were already
> present but which were never invoked.
> Let me know if this is acceptable for a test or if there's a need of a
> separate issue in the JBS.
I don't think a separate JBS issue for the extra test changes here is needed.
src/java.base/share/classes/java/lang/Math.java line 1501:
> 1499: // if the signs are the same and modulo not zero, round up
> 1500: if ((x ^ y) >= 0 && (q * y != x)) {
> 1501: return q + 1;
In `floorDiv()` this line is `r--` and there is only one return statement in
the method. I think the accepted convention is to return as soon as possible
like is done here, so perhaps it would be good to change `floorDiv()` to match?
In any cases the two should be consistent.
src/java.base/share/classes/java/lang/Math.java line 1612:
> 1610: // if the signs are the same and modulo not zero, adjust result
> 1611: if ((x ^ y) >= 0 && r != 0) {
> 1612: return r - y;
Similar comment as for `floorDIv()` above: `ceilMod()` does `mod += y` and
there is only one return.
-------------
PR: https://git.openjdk.java.net/jdk/pull/5285