[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