[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