JDK 15 RF(pre)R of JDK-8241374: add Math.absExact

Joe Darcy joe.darcy at oracle.com
Sun Mar 29 19:03:54 UTC 2020


Hi Stuart,

Full webrev for review include including tests:

     http://cr.openjdk.java.net/~darcy/8241374.0/

and the companion CSR:

     https://bugs.openjdk.java.net/browse/JDK-8241805

Replies interspersed below.

On 3/28/2020 7:37 PM, Stuart Marks wrote:
> Hi Joe,
>
> Overall this looks quite good. Thanks for being thorough about this; I 
> certainly would have forgotten about StrictMath. :-)

I may have initial forgotten about StrictMath myself on at least one 
prior occasion ;-)

> Since this is integer arithmetic, is it true that the StrictMath 
> versions are identical to the Math versions?

Right; conceptually StrictMath is a subclass of Math where certain 
methods have additional cross-platform reproducibility requirements. 
However, since all the methods are static, at least the javadoc gets 
replicated although the StrictMath implementations of integer methods 
can just call the Math one. (In that case, the delegation could go the 
other way too, but as Math is called more often, it is a little better 
to have the real implementation hosted there.)


>
> I have only a couple editorial quibbles.
>
>> +     * {@code int} range and an exception is thrown for that argument.
>
> Recommend adding a comma here:
>
>> +     * {@code int} range, and an exception is thrown for that argument.
>

Changed to


{@code int range}, so an exception is thrown for that argument.


> -- 
>
>> +     * @return the absolute value of the argument absent overflow
>
> I stumbled while reading this. While not incorrect, it seems a bit 
> awkwardly worded to me. How about something like "...the argument, 
> unless overflow occurs."

Changed as suggested.

Normally the exception behavior is not described in an @return tag, but 
the point of this method is its exceptional behavior so I think that 
variance with typical practice is fine.


>
> -- 
>
>> +            throw new ArithmeticException("Cannot represent " +
>> +                                          "absolute value of " +
>> +                                          "Integer.MIN_VALUE");
>
> In cases like this I usually prefer to put the entire string on its 
> own line, avoiding the concatenation:
>
>> +            throw new ArithmeticException(
>> +                "Cannot represent absolute value of 
>> Integer.MIN_VALUE");

Okay; also changed wording to:

     "Overflow to represent absolute value of Integer.MIN_VALUE"

Thanks,

-Joe



More information about the core-libs-dev mailing list