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 Wed, 29 Aug 2012 19:36:44 GMT
On 8/29/12 12:11 PM, Luc Maisonobe wrote:
> Le 29/08/2012 20:31, Sébastien Brisard a écrit :
>> Hello,
> Hi Sébastien,
>
>> 2012/8/29 Luc Maisonobe <Luc.Maisonobe@free.fr>:
>>> Le 29/08/2012 01:40, Gilles Sadowski a écrit :
>>>> Hi.
>>> Hello,
>>>
>>>>>> [...]
>>>>> I think I get your point, but again given transitive / nested
>>>>> dependencies I would not want to depend on it, even if all of the
>>>>> components have single-rooted exception hierarchies.  This is
>>>>> especially true if not all components adhere to the "wrap
>>>>> everything" rule - i.e., if they can generate and/or propagate RTEs
>>>>> that do not inherit from their base exception class.  From the
>>>>> standpoint of the caller, for example, what is the difference
>>>>> between [math]
>>>>>
>>>>> 0)  throwing IAE
>>>>> 1)  throwing MathIAE derived from IAE
>>>>> 2)  throwing MathIAE derived from MathRTE (base)
>>>>> assuming that [math] is not signing up to wrap and rethrow every
>>>>> exception - including IAE - we get from JDK classes?  Will the
>>> I was talking only about what we do throw ourselves.
>>>
>>>>> caller actually do anything different if the RTE is math-wrapped vs
>>>>> "naked" but coming out of the [math] code?  I understand that the
>>>>> try/catch may be several layers removed from the code calling a
>>>>> [math] API.
>>>>>
>>>>> Same applies to NPE, which we don't subclass now, but mostly handle
>>>>> as IAE.
>>>>>
>>>>> I guess one thing we might consider is trying to design for the
>>>>> invariant that we never propagate RTEs without wrapping.  But that
>>>>> would be a lot of work to retrofit and would have a performance cost.
>>>> I don't think that Luc means that we must wrap everyting in a home-made
>>>> exception.
>>> Gilles is right, I did not intend to catch and wrap everything.
>>>
>>>> Two possible cases for NPE:
>>>>  * The caller passed a "null" reference and will get, sooner or later, an
>>>>    NPE: it's a bug in his application.
>>>>  * An NPE was raised by a bug in CM, and we must fix it.
>>>>
>>>> I think that we should not check for "null" and thus have no use for an NPE
>>>> wrapper.
>>> I agree. We shouldn' wrap this.
>>>
>>>> Are there other "naked" exceptions that could come out of CM?
>>>> The policy of extensive precondition checking has the purpose (I think) to
>>>> prevent unexpected ("naked" or not) exceptions. If some slip through
>>>> nevertheless, doesn't it mean that we miss some check?
>>> I think we catch what we need to catch and already have a ggod deal of
>>> checking. Of course, we surely missed a few cases, we will fix them when
>>> they are identified.
>>>
>>>>>> Another problem is maintenance. Even if you consider the intermediate
>>>>>> developer did his work really accurately and managed to identify
all
>>>>>> exceptions thrown by the methods he calls in one version of Apache
>>>>>> Commons Math. When we change an error detection and decide that a
method
>>>>>> that did throw only MaxCountExceededException a method should throw
>>>>>> NumberIsToolLargeException instead (or in addition to the existing
one),
>>>>>> then the calling code would still compile, but the new exception
would
>>>>>> now go all the way upward. The two exceptions have no common ancestor
>>>>>> that can be catched, except Exception itself. With a single rooted
>>>>>> hierarchy, users can use some defensive programming: they can catch
the
>>>>>> common root and be safe when we change some internal details.
>>>>>>
>>>>>> A single root would also bring two things I find useful.
>>>>>>
>>>>>> The first useful thing is that the ExceptionContextProvider could
be
>>>>>> implemented at the root level, so we could retrieve this context
(in
>>>>>> fact, I sometime needs even to retrive the pattern and the arguments
>>>>>> from the context, and we also miss getters for that, but they are
easy
>>>>>> to add). It is not possible to catch ExceptionContextProvider because
it
>>>>>> is not a throwable (Throwable is a class, not an interface, so we
>>>>>> inherit the Throwable nature from the top level class, not as
>>>>>> implementing the ExceptionContextProvider interface.
>>>>>>
>>>>>> The second useful thing is for [math] development itself. With a
single
>>>>>> root, we can temporarily change its parent class from RuntimeException
>>>>>> to Exception, then fix all missing throws declaration and javadoc,
then
>>>>>> put the parent class back before committing. This would help having
more
>>>>>> up to date declarations. For now, I am sure we have missed a lot
of our
>>>>>> own exceptions and let them propagate upward without anybody knowing
it.
>>>> "let them propagate upward without anybody knowing it"
>>>> What do you mean? [Of course, all CM exceptions propagate upwards; that's
>>>> the purpose of raising them in the first place.]
>>>> Or did you just mean that the documentation is missing?
>>> I meant the documentation is missing.
>>>
>>>>>> As a test, I have just changed the parent for
>>>>>> MathIllegalArgumentException to Exception. I got 1384 compilation
>>>>>> errors. Just going to the first one (a constructor of
>>>>>> BaseAbstractUnivariateIntegrator), I saw we did not advertise the
fact
>>>>>> it may throw NumberIsTooSmallException and NotStrictlyPositiveException,
>>>>>> neither in a throws declaration nor in the javadoc. I did not look
at
>>>>>> the 1383 other errors...
>>>>> This is a good point.
>>>>>>> What I am missing is how knowing that an
>>>>>>> aspecific RTE came from within [math] makes a difference.  I
am
>>>>>>> skeptical about ever depending on that kind of conclusion because
>>>>>>> dependencies may bring [math] code in at multiple levels.  Also,
is
>>>>>>> there an implied assumption in your ideal setup that *no* exceptions
>>>>>>> propagate to [math] clients other than MRTE (i.e. we catch and
wrap
>>>>>>> everything)?
>>>>>> No, I don't make this assumption. I consider that at upper levels,
code
>>>>>> can receive exception from all layers underneath ([math] at the very
>>>>>> bottom, but also other layers in between). With two or three layers,
you
>>>>>> can still handle a few library-wide exceptions (see my example with
>>>>>> MathRuntimeException, and MylibException above). However, if at one
>>>>>> level the development rules state that all exception must be caught
and
>>>>>> wrapped (this happens in some critical systems contexts), then a
single
>>>>>> root hierarchy helps a lot.
>>>>> But if we allow some exceptions to propagate unwrapped, this does
>>>>> not work, unless I am missing the point here.
>>>> AIUI, when a CM exception is thrown, one (obviously) knows that CM threw
it.
>>>> When another exception (not a subclass of "MathRuntimeException") is thrown,
>>>> it did not come from a "throw" statement written in a CM source file.
>>> Right.
>>>
>>>>>> My point is that with a single root, we can get the best of two worlds:
>>>>>> large scope catches and pinpointed catches. The choice remains open
for
>>>>>> users. With a multi-rooted hierarchy, we force users to duplicate
the
>>>>>> same work for all exceptions we may throw, and we also force them
to
>>>>>> recheck everything when we publish a new version, even despite we
>>>>>> ourselves fail to document these exceptions accurately.
>>>>> We need to fix the documentation.  If going back to a single root
>>>>> makes automatic detection of gaps possible, that by itself is almost
>>>>> enough to get me to agree to go back to the single root.  Your
>>>>> arguments above (which I honestly only partially follow) are enough
>>>>> to make me +0 for this change.  I think I probably put too much
>>>>> weight on favoring standard exceptions when we are really only
>>>>> talking about "reinventing" a handful of them.
>>>> I think that there is no relationship between single root hierarchy and
>>>> fixing the documentation...
>>>> [Unless we mean to indiscriminately indicate
>>>> ---
>>>>   @throws MathRuntimeException if something goes wrong.
>>>> ---
>>>> everywhere.]
>>> Single root simplifies this. We hage to apply the trick only once.
>>>
>> I think we had a discussion a few months ago on how exceptions should
>> be documented. We came to no agreement at that time, although one
>> option (which I followed) was to
>>   - remove unchecked exceptions from the method's signature
>>   - add the unchecjked exceptions to the javadoc.
>> I agree we should make sure that all exceptions are advertised in the
>> javadoc. However, I don't see how Luc's trick can help us in this case
>> (if we agree that exceptions should *not* appear in the signature). Am
>> I missing something?
> No, you are not missing anything. Having only the javadoc without the
> declaration in the signature is a pain. I would prefer to keep both. 

+1

Just "going boom" with an unadvertised RTE is not acceptable
behavior.  We used to be very good about documenting and advertising
all exceptions generated by [math].  I would very much like to see
us get back to that level of API quality.  The fact that we can use
Luc's trick to verify that we have not missed anything is enough to
make me +1 for going back to a single root.

Phil


> As
> I said earlier in this thread, I gave up on the discussion at some time
> and did not participate to the last occurrence you point out. I thought
> I served a lost cause then.
>
> Luc
>
>
>> Sébastien
>>
>>
>> ---------------------------------------------------------------------
>> 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