JDK 9 RFR of JDK-4851777 Add BigDecimal sqrt method

[joe darcy]

Hi Brian,


On 5/20/2016 1:39 PM, Brian Burkhalter wrote:
> Hi Joe,
>
> I’ve a few comments / questions, mostly picayune.
>
> On May 19, 2016, at 11:43 PM, joe darcy <joe.darcy at oracle.com 
> <mailto:joe.darcy at oracle.com>> wrote:
>
>> Please review the addition of BigDecimal.sqrt:
>>
>>     JDK-4851777 Add BigDecimal sqrt method
>>
>> http://cr.openjdk.java.net/~darcy/4851777.5/ 
>> <http://cr.openjdk.java.net/%7Edarcy/4851777.5/>
>
> 2023      * <li> The square root of a number numerically equal to {@code
> Extraneous space after “<li>”.
> 2057              * The main steps of the algorithm below are as*follow*, first
> “follow” -> “follows”
> 2104             // root, it is helpful to instead normalize this*as*  so
> I think the “as” is unneeded.
> 2110             int scale = stripped.scale() - stripped.precision() + 1;
> The “+ 1” here is I suppose because in the referenced ACM paper they 
> work with a normalized fraction “f” in the range [0.1,1.0) whereas 
> here unscaledValue is a BigInteger. Then the analog of “f” in the 
> BigDecimal case is the variable “working.”

Right; BigDecimal values are naturally normalized with the decimal point 
to the right of the digit block as opposed to the left of the digit 
block (meaning the value is fractional) as is commonly done in binary 
floating-point. To make sure the value is in the range for double, since 
BigDecimal can encode truly huge values that would overflow, I normalize 
the working/f BigDecimal to be a fraction.

> 2226      * the computed result for the chosen rounding mode .
> Extraneous space before period.

Grammatical error will be fixed being pushing.

>
> In terms of additional testing, did you contemplate comparing the 
> consistency of the results versus BigInteger.sqrt()? Obviously one 
> would not be looking for equality in most cases.

That is a natural extension and a fine idea for future additional 
testing :-)

>
> All in all I think this looks good insofar as my reading can tell.
>
> Reviewed +1.
>
>

Thanks,

-Joe