[9] RFR of 8032027: Add BigInteger square root methods
Joseph D. Darcy
joe.darcy at oracle.com
Wed Dec 2 03:25:00 UTC 2015
Hi Brian,
On 11/30/2015 3:24 PM, Brian Burkhalter wrote:
> Hi Joe,
>
> On Nov 29, 2015, at 10:01 AM, joe darcy <joe.darcy at oracle.com
> <mailto:joe.darcy at oracle.com>> wrote:
>
>> The "if (...) " logic that is repeated a few times in this method
>> could be pulled out into its own method, possibly one structured a
>> bit differently to return the number of errors.
>>
>> I think it would be acceptable to push the tests in their current
>> state, but I would prefer to see a little more refactoring.
>>
>> I would have expected some tests to directly work off of the
>> definition of integer square root, namely that k^2 is less or or
>> equal to the argument but (k+1)^2 is larger.
>
> I have updated the patch per your comments above. The new version is here:
>
> http://cr.openjdk.java.net/~bpb/8032027/webrev.03/
> <http://cr.openjdk.java.net/%7Ebpb/8032027/webrev.03/>
>
> Thanks,
>
> Brian
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.
Thanks,
-Joe
More information about the core-libs-dev
mailing list