[9] RFR of 8032027: Add BigInteger square root methods
Brian Burkhalter
brian.burkhalter at oracle.com
Wed Nov 25 20:02:45 UTC 2015
On Nov 16, 2015, at 10:00 AM, joe darcy <joe.darcy at oracle.com> wrote:
> Returning to this review…
Likewise …
Please refer to the updated patch at http://cr.openjdk.java.net/~bpb/8032027/webrev.02/.
> A few comments and suggestions:
>
> 2452 public BigInteger[] sqrtAndRemainder() {
> 2453 BigInteger s = sqrt();
> 2454 BigInteger r = this.subtract(s.square());
> 2455 return new BigInteger[] {s, r};
> 2456 }
>
> It that "remainder >= 0" would be a good assert to add before line 2455.
Assert added.
> As a stylistic matter, in MutableBigInteger I would prefer a direct check of bitLength rather than relying on the exception being thrown from longValueExact. Something like
>
> if (bitLength < 63)
> // use double initial approx
> else
> // use other technique
Changed as suggested.
> In the tests, use of lambda could allow the core testing logic to be shared.
I don’t know precisely what was expected but the test has been modified to reduce redundant code.
Thanks,
Brian
More information about the core-libs-dev
mailing list