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: Single root for Exceptions
Date Tue, 28 Aug 2012 23:40:49 GMT
Hi.

> > [...]
> 
> 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
> 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.

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.

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?

> 
> > 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?

> > 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.

> >
> > 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.]


Gilles

> [...]

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


Mime
View raw message