[9] RFR of 8032027: Add BigInteger square root methods

[Joseph D. Darcy]

Hi Brian,

New version looks good.

One more case to try: start with a BigInteger that would overflow to 
Double.POSITIVE_INFINITY when the doubleValue method was called. If this 
case doesn't take too long to run, it would be a fine additional case to 
add to the test. 2^1024 should be fine input value. More precisely,

      (new BigDecimal(Double.MAX_VALUE)).toBigInteger().add(BigInteger.ONE);

should do the trick. If the code passes with this value, you're okay to 
push. Well, while you're at it, might as well verify

     (new BigDecimal(Double.MAX_VALUE)).toBigInteger()

behaves well too ;-)

Thanks,

-Joe

On 12/9/2015 5:41 PM, Brian Burkhalter wrote:
> Hi Joe,
>
> On Dec 1, 2015, at 7:25 PM, Joseph D. Darcy <Joe.Darcy at Oracle.Com 
> <mailto:Joe.Darcy at Oracle.Com>> wrote:
>
>> Current version looks okay. One more request, before pushing please 
>> add explicit test cases for the for the largest number having 63 bits 
>> and the smallest number having 64 bits. No need for another round of 
>> webrevs for that.
>
> Well there is after all a need for another round of review:
>
> http://cr.openjdk.java.net/~bpb/8032027/webrev.04/ 
> <http://cr.openjdk.java.net/%7Ebpb/8032027/webrev.04/>
>
> That was a good call to add the above tests: one of them failed. This 
> was found to be due to a floor() where there should have been a ceil().
>
> Summary:
>
> MutableBigInteger: at line 1920 change Math.floor(.) to Math.ceil(.).
> BigIntegerTest: at lines 331-340 add testing of 2^N and 2^N - 1, 0 < N 
> < 1024
>
> Thanks,
>
> Brian