Hi Daniel
2006/8/25, Daniel Fridlender <[EMAIL PROTECTED]>:
On 8/23/06, Mikhail Loenko <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>:
> > Hi Mikhail,
> >
> > On 8/17/06, Mikhail Loenko <[EMAIL PROTECTED]> 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) <[EMAIL PROTECTED]>:
> > > > [ 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: [EMAIL PROTECTED]
> > > For additional commands, e-mail: [EMAIL PROTECTED]
> > >
> > >
> >
> > ---------------------------------------------------------------------
> > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > To unsubscribe, e-mail: [EMAIL PROTECTED]
> > For additional commands, e-mail: [EMAIL PROTECTED]
> >
> >
>
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
>
>
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
