[9] RFR of 8066842: java.math.BigDecimal.divide(BigDecimal, RoundingMode) produces incorrect result

[Paul Sandoz]

On Feb 6, 2015, at 2:43 AM, Brian Burkhalter <brian.burkhalter at oracle.com> wrote:

> Hi Paul,
> 
> On Feb 5, 2015, at 6:06 AM, Paul Sandoz <paul.sandoz at oracle.com> wrote:
> 
>> I don't claim to understand the fine details of these methods but i can see how the new method avoid loosing bits.
>> 
>> 4947     private static long[] divRemNegativeLong(long n, long d) {
>> 4948         if (n >= 0) {
>> 4949             throw new IllegalArgumentException("Non-negative numerator");
>> 4950         } else if (d == 1) {
>> 4951             return new long[]{0, n};
>> 4952         }
>> 
>> Why not use an assert instead of an IAE since this is an internal method.
> 
> I had actually wondered whether this could just be a “trusted” method since the check of the first parameter is already done in the calling code, then the if-branch could be removed altogether, but I suppose the assert is safer.

Yes, asserts are good here, they will be enabled when testing.

> 
>> Also the case of d ==1 could be pulled out just like for the case of tmp being +ve: 
>> 
>>  if (v1 == 1) {
>>    q1 = tmp;
>>    r_tmp = 0;
>>  } else if (tmp >= 0) {
>>    ...
>>  } else {
>>    ...
>>  }
>> 
>> then the asserts would be:
>> 
>>  assert n < 0 :  n;
>>  assert d != 1 : d;
> 
> Thanks for the suggestions. Please see updated patch:
> 
> http://cr.openjdk.java.net/~bpb/8066842/webrev.01/
> 

+1, but you should remove the "@throws IAE" on divRemNegativeLong before you push.

Paul;.