On Fri, 27 Aug 2021 18:53:23 GMT, Raffaello Giulietti <github.com+70726043+rgiulie...@openjdk.org> 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