patch ping Re: [java] Fix BigDecimal ROUND_HALF_EVEN

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/