commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sébastien Brisard <sebastien.bris...@m4x.org>
Subject Re: [math] Re: Single root for Exceptions
Date Wed, 29 Aug 2012 18:31:43 GMT
Hello,

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?

Sébastien


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


Mime
View raw message