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.da...@oracle.com
<mailto:joe.da...@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
