On 12/21/15 8:21 AM, Gilles wrote:
> 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").
In 3.x, MIAE extends IAE and we have all the context stuff working.
I guess you are concerned about the "duplicated code" in the
multiple roots extending the standard exceptions. Here is the full
extent of it:
/**
* @param pattern Message pattern explaining the cause of the error.
* @param args Arguments.
*/
public MathIllegalArgumentException(Localizable pattern,
Object ... args) {
context = new ExceptionContext(this);
context.addMessage(pattern, args);
}
/** {@inheritDoc} */
public ExceptionContext getContext() {
return context;
}
/** {@inheritDoc} */
@Override
public String getMessage() {
return context.getMessage();
}
/** {@inheritDoc} */
@Override
public String getLocalizedMessage() {
return context.getLocalizedMessage();
}
I see that as NOT_A_PROBLEM. In 3.x, we have 6 classes that need
this little bit of similar code. That is after 10 years of
development. I doubt we will have more than a few more needed at
the top level.
>
> 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.
Meaningful exception messages are a best practice in Java. They are
a very useful aid in debugging and production support.
>
> 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.
Easiest is if the library follows the Java best practice, which is
to favor standard exceptions. Then these can be caught without
rummaging through library-specific javadoc to figure out what
unchecked exception means what. This is especially true for
unchecked exceptions.
>
> 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.
I think we can have it both ways. The 3.x setup allows us to extend
standard exceptions *and* provide good exception messages.
>
> 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).
Well, we have a nice example in front of us in our own test case
here. But the bigger deal is the pain for every user who when
migrating from 3.x to 4 has to stop catching standard IAE if they
did this and replace it with MIAE. Note that the compiler will be
no help there - failure to "comply" will cause the client app to "go
boom."
Phil
>
> In conclusion, let's not jump to conclusions. ;-)
>
>
> Gilles
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
