commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <phil.ste...@gmail.com>
Subject Re: [math] Re: Single root for Exceptions
Date Mon, 03 Sep 2012 03:22:40 GMT
On 9/2/12 7:35 PM, Sébastien Brisard wrote:
> Hi,
> here is another problem. In
> MatrixUtils.createFieldIdentityMatrix(Field<T>, int), the constructor
> Array2DRowFieldMatrix(Field<T>, T[][], boolean) is called. This
> constructor throws a DimensionMismatchException if the data array is
> not rectangular. This is never going to occur in
> createFieldIdentityMatrix, so what should we do
> 1. Populate the throws clause of createFieldIdentityMatrix with
> DimensionMismatchException anyway?
> 2. try/catch with empty catch, which I find ugly?

Neither.  The exception will (provably) never be thrown.  Therefore
there is no reason to advertise it.
Just allow it as an exception when applying "rhymes with truc's" trick.

What we need to accomplish is to effectively advertise all
exceptions that a caller may actually see.  In some cases,
superclasses of the exceptions being propagated may be appropriate
and in extreme cases, due to holes in the hierarchy or deep nesting
of the API, the only thing that will make sense to the caller is
MRTE.  This should be a last resort.

Phil
>
> Sébastien
>
> 2012/9/3 Sébastien Brisard <sebastien.brisard@m4x.org>:
>> Hi,
>>
>> 2012/9/1 Gilles Sadowski <gilles@harfang.homelinux.org>:
>>> 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.
>>>
>> OK, but the code is going to become a total clutter anyway. Indeed, in
>> PreconditionedIterativeLinearSolver, there are a bunch of solve()
>> method, all based on the abstract method
>>     public abstract RealVector solveInPlace(RealLinearOperator a,
>>                                             RealLinearOperator m,
>>                                             RealVector b,
>>                                             RealVector x0)
>>
>> In conjugate gradient, the above method throws a
>> NonPositiveDefiniteOperatorException. This means that in the
>> implementation of ConjugateGradient, I need to override every single
>> solve method that are implemented in the base class, just to add the
>> proper throws clause. At this point, I have stopped "populating the
>> throws clause", as I am not sure it's the best way to go.
>> S
>>> 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
>>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


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


Mime
View raw message