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