commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles <gil...@harfang.homelinux.org>
Subject Re: [Math] Exceptions from "JDKRandomGenerator"
Date Mon, 21 Dec 2015 15:21:33 GMT
On Mon, 21 Dec 2015 06:06:16 -0700, Phil Steitz wrote:
> On 12/20/15 8:41 PM, Gilles wrote:
>> On Sat, 19 Dec 2015 11:35:26 -0700, Phil Steitz wrote:
>>> On 12/19/15 9:02 AM, Gilles wrote:
>>>> Hi.
>>>>
>>>> While experimenting on
>>>>   https://issues.apache.org/jira/browse/MATH-1300
>>>> I created a new
>>>>   JDKRandomGeneratorTest
>>>> that inherits from
>>>>   RandomGeneratorAbstractTest
>>>> similarly to the classes for testing all the other RNG implemented
>>>> in CM.
>>>>
>>>> The following tests (implemented in the base class) failed:
>>>> 1.
>>>>
>>>> 
>>>> testNextIntNeg(org.apache.commons.math4.random.JDKRandomGeneratorTest)
>>>>
>>>> Time elapsed: 0.001 sec  <<< ERROR!
>>>>    java.lang.Exception: Unexpected exception,
>>>>
>>>> 
>>>> expected<org.apache.commons.math4.exception.MathIllegalArgumentException>
>>>>
>>>> but was<java.lang.IllegalArgumentException>
>>>> 2.
>>>>
>>>> 
>>>> testNextIntIAE2(org.apache.commons.math4.random.JDKRandomGeneratorTest)
>>>>
>>>> Time elapsed: 0.015 sec  <<< ERROR!
>>>>    java.lang.IllegalArgumentException: bound must be positive
>>>>
>>>> This is caused by try/catch clauses that expect a
>>>> "MathIllegalArgumentException"
>>>> but "JDKRandomGenerator" extends "java.util.Random" that for those
>>>> methods throws
>>>> "IllegalArgumentException".
>>>>
>>>> What to do?
>>>
>>> I would change the test to expect IllegalArgumentException.  Most
>>> [math] generators actually throw NotStrictlyPositiveException here,
>>> which extends MIAE, which extends IAE, so this should work.
>>
>> It turns out that, in the master branch, the hierarchy is
>>
>>        RuntimeException
>>              |
>>      MathRuntimeException
>>              |
>>    MathIllegalArgumentException
>>
>> as per
>>   https://issues.apache.org/jira/browse/MATH-853
>>
>> [And the Javadoc and "throws" clauses are not yet consistent with
>> this
>> in all the code base (e.g. the "RandomGenerator" interface).]
>>
>> So, in 4.0, "JDKRandomGenerator" should probably not inherit from
>> "java.util.Random" but delegate to it, trap standard exceptions
>> raised,
>> and rethrow CM ones.
>
> I guess that is the only way out if we are going to stick with the
> MATH-853 setup.  This example illustrates a drawback identified in
> the thread mentioned in the ticket.  I would personally be happy
> reverting master back to the 3.x setup.

We can't throw (!) the many discussions that led to MATH-853 by just
reverting on the basis of the current issue.

Perhaps that the multiple hierarchies are better, maybe not.
But we have to figure out arguments that are more convincing than
nostalgia.

For instance, IMO, we have contradicting wishes:
* Have CM exception inherits from the JDK ones with the same semantics.
* Have all CM exceptions provide the same additional functionality (the
   "ExceptionContext").

The first looks nice from a user POV (as in "I know this already").
The second is an internal requirement to avoid code duplication.

IMO, it always comes back to those opposite views we have concerning 
the
place where human-readable messages should be generated: my view is 
that
if a low-level library message bubbles up to the console, then it's not
our fault, but the application programmer's.

Please assume that for a moment; then CM could have its algorithms 
throw
some internal subclass of "MathRuntimeException" (whose type is known 
to
the caller) and the caller can decide whether he must trap it in order
to rethrow a standard type, or his own type (tailored to the context 
which
he knows but CM doesn't).

In that scenario, it is much easier for the caller when the low-level
library's exceptions hierarchy is unique (especially when he uses 
several
libraries).
And it's also easier for us because we can avoid code duplication.

You never convinced me that you were aware of this type of scenario and
why, somehow, it was trumped by a nicely formatted message on the 
console.
The more so that a single hierarchy does not prevent the latter:
at the point where the message is printed, the exception type is 
useless.

To summarize, if the top-level message matters most, then I'd be -1 to 
revert
(because that's an orthogonal issue); if having standard types matters 
most,
I'd wait for use-cases that show how these types are used in the 
caller's
code before trying to figure out how to satisfy them (while taking our
internal requirements in to account too).

In conclusion, let's not jump to conclusions. ;-)


Gilles


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message