commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles Sadowski <gil...@harfang.homelinux.org>
Subject Re: [math] Re: Single root for Exceptions
Date Sat, 01 Sep 2012 10:21:08 GMT
Hello.

> >>>
> >>> in ConjugateGradient, I get the following error
> >>>
> >>> "Exception NonPositiveDefiniteOperatorException is not compatible with
> >>> throws clause in
> >>> PreconditionedIterativeLinearSolver.solveInPlace(RealLinearOperator,
> >>> RealLinearOperator, RealVector, RealVector)"
> >>>
> >>> This comes from the fact that general iterative solvers do not require
> >>> positive definite operators (therefore, no
> >>> "throws NonPositiveDefiniteOperatorException" clause in the signature of
> >>> PreconditionedIterativeLinearSolver.solveInPlace), while conjugate
> >>> gradient does.
> >>>
> >>> Do I need to add the "throws NonPositiveDefiniteOperatorException"
> >>> clause in the signature of
> >>> PreconditionedIterativeLinearSolver.solveInPlace as well?
> >>
> >> I think so, as users may use a ConjugateGradient and store it in a
> >> variable declared with the base class only. However, I don't know if
> >> adding an unchecked exception to the signature of an interface is a
> >> compatible change or not.
> >>
> >> Luc
> >>
> > clirr does not seem to be complaining, so I'll do as you say. Here is
> > what I'm planning to add to the javadoc of the mother abstract class.
> > What do you think?
> > 
> >      * @throws NonPositiveDefiniteOperatorException if {@code a} or {@code m}
> >      * is not positive definite (required by some iterative solvers)
> 
> Perfect!

I don't agree on the principle that base classes/interfaces should be aware
of the detailed requirements of their implementations.
We can foresee that subclasses might need to throw _some_ kind of exception
when something (yet unknown at the time of the interface design) goes wrong.
But base classes (or interfaces) should not have to change when a new class
extends (or implements) it.

For cases like the above, we must create a parent exception (or use an
existing one if it is appropriate) in order to convey that subclasses
_might_ throw exceptions that inherit from that one.
It this particular case, it seems that operators that are not positive
definite are illegal for method "solveInPlace", hence we conclude (not
surprisingly) that "MathIllegalArgumentException" can be thrown from classes
that implement "RealLinearOperator.solveInPlace".

In fact, this is even too much of a "guessing" work. And this where a single
root can be used to clearly advertize that, by design, CM is supposed to
throw only (subclasses of) "MathRunimeException". _That_ exception, and
_nothing_ else should really be inserted in the "throws" clause of upper
level base classes and interfaces.

Referring to the above example, it is the duty of the user of CM to read the
documentation of the (concrete) classes he uses if he wants to know which
subclass of "MathRuntimeException" may actually be thrown; it is not enough
to read the documentation of the interface!
To be clear, CM will become a total clutter if upper levels attempt to
report everything all the way down. This is in blatant contradiction with
the principle (or rule, or best practice) of encapsulation.


Best regards,
Gilles

> > [...]

P.S. Practically, at this point, I propose to not touch interfaces or base
     classes, but only add the "throws" clauses where the methods actually
     throw the exceptions, and in some cases, one level up (where it is
     still obvious that a particular condition is required).
     Of course, that makes it impossible to test the "switch to checked
     exception" as the work is in progress. For this, let's wait until all
     the first pass has been completed, then we can see what is missing,
     and decide while seeing the picture.

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


Mime
View raw message