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.da...@oracle.com <mailto:joe.da...@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

Reply via email to