harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mikhail Loenko" <mloe...@gmail.com>
Subject Re: [jira] Assigned: (HARMONY-1208) Bug fixing and cosmetics for Harmony 935
Date Fri, 25 Aug 2006 03:41:19 GMT
Hi Daniel

2006/8/25, Daniel Fridlender <dfridlender@gmail.com>:
> On 8/23/06, Mikhail Loenko <mloenko@gmail.com> wrote:
> > Hi Daniel,
> >
> > I've tried the patch. 4 math tests failed. Could you please take a look?
> >
> > Thanks,
> > Mikhail
> >
>
> Hi Mikhail,
>
> You are right.  Two of these four tests are due to typos in the
> exception messages:
>
> 1)org.apache.harmony.tests.java.math.BigDecimalConstructorsTest.testConstrStringExponentIntegerMin(
> BigDecimalConstructorsTest.java:497)
> expected<"Scale out of range."> but was <"Scale out of range">
>
> 2)org.apache.harmony.tests.java.math.BigDecimalScaleOperationsTest.testMovePointRightException(
> BigDecimalScaleOperationsTest.java:335)
> expected<"Underflow"> but was <"UnderFlow">
>
> We corrected them (and a similar problem with "Overflow") with a new
> patch "typos.diff.zipĀ“" in H1208.
>
> The other two failing tests require some discussion.
>
> 3)org.apache.harmony.tests.java.math.BigDecimalConstructorsTest.testConstrBI(BigDecimalConstructorsTest.java:75)
>
> I believe the test is wrong in this case since according to the API
> specification the NullPointerException will actually be thrown before
> the try block.  The test incorrectly expects it to be thrown in the
> block.
>
> The RI also fails this test.

I've just rerun it on RI and it passed. What RI version do you use?

I got the argument about the spec. It in a class description, right?


> 4)org.apache.harmony.tests.java.math.BigDecimalArithmeticTest.testDivideByZero(BigDecimalArithmeticTest.java:492)
> expected <"BigInteger divide by zero"> but was <"Division by zero">
>
> This is not just a typo.  Both exception messages are used by the RI
> in different BigDecimal.divide methods when dividing by
> BigDecimal.ZERO.  There is at least a third message: "/ by zero".
> Some methods seem to use different messages depending on the size of
> the dividend.
>
> So, I see two possibilities here:
>
> a) use everywhere the message "Division by zero".
> b) try to find out when RI uses each of the different messages.
>
> I would suggest a) rather than b) since from the point of view of the
> programmer it is reasonable to obtain the same message when dividing
> by BigDecimal.ZERO regardless the size (or internal representation) of
> the dividend and the method invoked for the division.

In general if a programmer use only one method in their implementation where
RI throws "BigInteger divide by zero", then they are not aware of
other exception
messages and they might hard code that "BigInteger divide by zero" one.

So I'd prefer to keep it look like RI as it is now. BUT, if this change implies
an undesirable design change then I'm OK with discrepancy in exception
messages

Thanks,
Mikhail


>
> In addition, doing b) may imply inspecting the behavior of RI a bit
> too far.  I am reluctant to do this since different messages may
> reveal when the unscaled value is implemented as a BigInteger and when
> as a primitive integral value.
>
> We submitted a new patch "tests.diff.zip" to H1208 correcting the test
> mentioned in 3) and replacing the test in 4) by another invocation to
> divide with the "Division by zero" message.
>
> Both RI and our implementation pass the tests.
>
> I think both patches should be applied.  In the meantime we may
> discuss if the approach b) is preferred.
>
> Regards,
>
>                        Daniel
>
> > 2006/8/22, Daniel Fridlender <dfridlender@gmail.com>:
> > > Hi Mikhail,
> > >
> > > On 8/17/06, Mikhail Loenko <mloenko@gmail.com> wrote:
> > > > Hi Daniel,
> > > >
> > > > Please resubmit the patch with license granted to ASF
> > >
> > > done.
> > >
> > > >
> > > > Seems like the patch was generated against original H-935 contribution
> > > > and does not take into account later changes. Could you please regenerate
the
> > > > patch?
> > >
> > > The new patch should be applied to the version of java.math currently
> > > in the svn repository.  It includes performance update from Vladimir
> > > (with bugs fixed) and other similar optimizations.  We tried to
> > > respect spacing and line breaking from the version of the repository
> > > (it often disagrees with the original H935).
> > >
> > > Thanks,
> > >
> > > Daniel
> > >
> > > >
> > > > Thanks,
> > > > Mikhail
> > > >
> > > > 2006/8/17, Mikhail Loenko (JIRA) <jira@apache.org>:
> > > > >     [ http://issues.apache.org/jira/browse/HARMONY-1208?page=all
]
> > > > >
> > > > > Mikhail Loenko reassigned HARMONY-1208:
> > > > > ---------------------------------------
> > > > >
> > > > >    Assignee: Mikhail Loenko
> > > > >
> > > > > > Bug fixing and cosmetics for Harmony 935
> > > > > > ----------------------------------------
> > > > > >
> > > > > >                 Key: HARMONY-1208
> > > > > >                 URL: http://issues.apache.org/jira/browse/HARMONY-1208
> > > > > >             Project: Harmony
> > > > > >          Issue Type: Improvement
> > > > > >            Reporter: Daniel Fridlender
> > > > > >         Assigned To: Mikhail Loenko
> > > > > >            Priority: Minor
> > > > > >         Attachments: MathDiff.diff.zip
> > > > > >
> > > > > >
> > > > > > A patch to fix some bugs in Harmony 935, turn serialization
compatible with RI and some cosmetics.
> > > > >
> > > > > --
> > > > > This message is automatically generated by JIRA.
> > > > > -
> > > > > If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
> > > > > -
> > > > > For more information on JIRA, see: http://www.atlassian.com/software/jira
> > > > >
> > > > >
> > > > >
> > > >
> > > > ---------------------------------------------------------------------
> > > > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > > > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> > > > For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> > > >
> > > >
> > >
> > > ---------------------------------------------------------------------
> > > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> > > For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> > >
> > >
> >
> > ---------------------------------------------------------------------
> > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> > For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> >
> >
>
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>
>

---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org


Mime
View raw message