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