patch ping Re: [java] Fix BigDecimal ROUND_HALF_EVEN

Per Bothner per@bothner.com
Tue Jul 1 06:01:00 GMT 2003


Jerry Quinn wrote:

> Hey, folks.  Could someone take at this?  I had posted a modified
> patch for fixing the above in
> http://gcc.gnu.org/ml/java-patches/2003-q2/msg00392.html

Sorry for letting this slide.

Here are my comments from looking at the code.

The multiplication by two in the calculation of half would be more
efficient as a shiftLeft(1).

It probaby would be better to avoid calculating half when it is not
needed.  That does make the logic slightly uglier.  Perhaps:


     switch (roundingMode)
       {
       case ROUND_UNNECESSARY:
	throw new ArithmeticException ("newScale is not large enough");
       case ROUND_CEILING:
	roundingMode = (sign == 1) ? ROUND_UP : ROUND_DOWN;
	break;
       case ROUND_FLOOR:
	roundingMode = (sign == 1) ? ROUND_DOWN : ROUND_UP;
	break;
       case ROUND_UP:  case ROUND_DOWN:
         break;
       default:
         half = ...;
         switch (roundMode)
           ...
      }

This (in the original code) looks questionable:
     int sign = unrounded.signum ();
Consider the test in ROUND_CEILING:
	roundingMode = (sign == 1) ? ROUND_UP : ROUND_DOWN;
Shouldn't this be whether the this.signum() is positive?
Consider dividing -9 by 10.  unrounded.signum () is 0,
so we round down when we should be rounding up.

More generally, have you considered what happens when val
is negative?  The code might work - or it might now.  I haven't
tried to analyze what happens.  Have you tested it?
-- 
	--Per Bothner
per@bothner.com   http://per.bothner.com/




More information about the Java mailing list