On Nov 16, 2015, at 10:00 AM, joe darcy <joe.da...@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

Reply via email to