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

Reply via email to