This is the mail archive of the
java@gcc.gnu.org
mailing list for the Java project.
Re: patch ping Re: [java] Fix BigDecimal ROUND_HALF_EVEN
- From: Per Bothner <per at bothner dot com>
- To: Jerry Quinn <jlquinn at optonline dot net>
- Cc: java at gcc dot gnu dot org
- Date: Mon, 30 Jun 2003 23:03:41 -0700
- Subject: Re: patch ping Re: [java] Fix BigDecimal ROUND_HALF_EVEN
- References: <E19MZLG-0001QR-00@localhost> <87ptlssd21.fsf@fleche.redhat.com> <3EE605DA.30209@bothner.com> <16128.59875.171137.258646@gargle.gargle.HOWL>
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/