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