On Mon, 21 Dec 2015 10:01:33 -0700, Phil Steitz wrote:
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.
It's not a problem of how many lines of code must be duplicated.
The problem is the duplication.
Duplication is not a best practice AFAIK.
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.
We do not favour standard exceptions because we _have_ to define our
own.
And we have done this for various "good" reasons in line with other
best practices, subject to internal requirements.
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.
A standard exception does not mean anything specific.
So why bother with the "implementation detail"?
try {
/** any code that deep down may or may not call CM */
} catch (RuntimeException e) {
// Deal with it or rethrow.
}
will work, always. User does not need to know anything to print
and read the meaningful message, only JDK's "RuntimeException" here.
The problem is _not_ there.
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.
The question is: Why extending standard exceptions?
We came to another answer in MATH-853.
So the problem cannot be solved as if it did not exist.
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."
All this has been said again and again.
I'd like that we do not take our own assumptions for granted
(be they the scenario which I described, or that compatibility
is the ultimate goal, even if the design is crap) but instead
that we could talk about what a live project CM can be, looking
to what can be done in possibly novel ways.
A new major version is an opportunity to legally break
compatibility in order to try new things.
Instead we have full-time discussions to avoid changing anything.
Users are never forced to follow development.
Gilles
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
