xmlgraphics-fop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeremias Maerki <dev.jerem...@greenmail.ch>
Subject Re: Throwing derivatives of RuntimeException
Date Sat, 04 Jan 2003 20:36:50 GMT
Hi Joerg

Ok, now we're talking.

On 04.01.2003 19:48:25 Joerg Pietschmann wrote:
> On Saturday 04 January 2003 15:59, Jeremias Maerki wrote:
> > Hi Joerg
> Oops! Sorry, I should have checked twice before committing to the
> wiki, of course its not the PS renderer but the TTFReader and PFMReader
> which throw CascadingRuntimeExceptions. I admit this makes the case
> less relevant.

Right. I'm not really proud of what I did there. But I thought it
doesn't matter so much since those are (only) little command line tools.
Look at what I changed. The exception in PFMReader you mention below was
a simple e.printStackTrace() before. I didn't want to have error logging
in various places.

http://cvs.apache.org/viewcvs.cgi/xml-fop/src/org/apache/fop/fonts/apps/PFMReader.java.diff?r1=1.9&r2=1.10&diff_format=h

I wouldn't have done things like this in other parts of FOP. At least, I
hope I haven't. I promise to fix these uglies as soon as my local FOP
compiles again.

> Also, never say never. I meant: Throw subclasses of RuntimeException
> with caution. There is a reason for the "throws" decl. Well it may be seen
> a bit devalued if every method throws a FOPException.
> Runtime exceptions should in general only be thrown by low-level, technical
> and widely used classes, where the user of the class can be reasonably
> expected to check the conditions causing runtime exceptions beforehand
> (i.e. being prudent enough to avoid them on average), and where a more
> specific exception would be seen misplaced in at least some contexts the
> class is used. Also, conditions for Runtime Exceptions should be
> *deterministic*, i.e. only depend on the context completely under the
> control of the user, preferably only dependent on the data passed to the
> routine throwing the exception. An IOException is *not* deterministic in this
> sense: someone else could have deleted the file the routine is asked to
> open in the meantime.
> Also, never should a non-runtime exception be caught and a runtime
> exception be thrown: (PFMReader.java)
>   } catch (Exception e) {
>     throw new CascadingRuntimeException("Error
>         while serializing XML font metric file", e);

With that I fully agree and usually apply these rules to my work. :-)

RuntimeExceptions such as IllegalArgumentException or UnsupportedOperationException
are very good tools to tell a programmer that has done something wrong.
Also NullPointerExceptions with no message occurs still too often in FOP
IMO.

> There is always a certain tradeoff when designing which exceptions are
> thrown:
> - Unifying each and every exception into one class or into classes of a
>   single inheritance tree keeps the "throws" decl short, but then this decl
>   respective the exception types (class names) no longer carry much
>   information. Duplicating existing exception types into said hierarchy
>   (like FOPIOException, FOPFileNotFoundException...) is nowadays
>   out of fashion (it was often seen in C++, because there was no common
>   IOException there).
>   With exception unification you are basically forced to cascade, and
>   unwinding the exception cascade may make long and confusing log
>   entries.

I don't think they are confusing if you know the concept. They may get
long, but they are soooo helpful sometimes. And I prefer a long log
entry over an Exception of which I can't determine the origin.

> - Throwing a lot of unrelated exceptions may cause "throws" decls
>   to grow into unwieldy beings. Currently, there usually aren't that
>   much unrelated inheritance trees of exceptions, however, using
>   more and more libraries of different origin and design this can become
>   an issue.

In my experience it's usually possible to combine them (or some of them) using
a derived CascadingException with no negative impact.

> - Throw a RuntimeException. Such exceptions don't need to be declared.
>   This can be seen positive: the user *usually* should not be worried that
>   something can go wrong. It can also be seen negative: the user is
>   deprieved of the information that something can go wrong, and the
>   compiler is silenced about this fact.
> 
> So, if the input file is not found, what to do?
> 1. Nothing, just let the FileNotFoundException pass up. Add "throws
>   IOException".
> 2. Catch it and throw an IllegalArgumentException("wrong filename"+...)
>    (Ok, stretching credibility, but let's pass it for now). Add nothing
>   to the throws decl.
> 3. Catch it and throw a FOPException("FO file not found"+...).
> 
> Pick one, formulate arguments why you picked it and formulate guidelines
> to make sure the same answer would be picked in similar situations (for
> some definition of "similar").
> Also, make up more such examples :-)

I'd say that 1 should be followed as long as it makes sense and then
maybe switch to 3. Well, the latter is probably not applicable in this
particular example because IOExceptions are likely to happen in a
component such as FOP.

> > I'd like to go with a mixture of your first and third point.
> This means
> - Derive a FOPException from avalon.CascadingException.
> - Throw *only* java.lang.* exceptions or subclasses of FOPException.
> This means:
> - the throws decl features only FOPException (or a subclass) and
>   java.lang.* exceptions
> - each time a method of a class not from a FOP package is called,
>   every non-java.lang.* exception *must* be caught and either a
>   FOPException (or a subclass) or a java.lang exception must be thrown.
> This seams to be a bit too stringent, probably some slack is called for:
> - Allow for SAXExceptions.

...and other important Exceptions. Apply common sense but have a good
reason ready. :-)

> - Allow for other Avalon exceptions, in particular other subclasses of
>   CascadingException. Well, I'm not sure I'd like this, but using more
>   and more Avalon libraries (or XML commons or whatever the project
>   is currently called) might make this necessary.

I wouldn't highlight Avalon Exceptions because the only thing they do is
adding cascading functionality. If this were already implemented in JDK
1.2 there wouldn't have been a need for these Exceptions.

> Could you describe your model of this mixture in more detail?

With my comments you're pretty much there. I'd like to point out a few
points that are I think are important in this context:
- Choose the right Exception in a particular place. I think we already
  outlined most of the guidelines in out exchange.
- The Exception must hold as much information as possible to help the
  consumer (user, developer, code) determine the next steps.
- When an Exception is handled and rethrown, no information must be lost,
  even if the stacktrace get's big.

It would obviously make sense to come up with a good set of clearly
formulated guidelines for exception handling, but that is something that
IMO has to be done in another context. I don't want to blow up our Style
Guide too much, because too much doesn't get read. If we had a good URL
to a page about best practices with Exceptions I wouldn't mind adding
them. I think it would make sense to bring this topic up in the
community mailing list so all Java-addicted could come up with a nice,
concise Wiki-page. I haven't found anything on the net that also
includes nested exceptions. What do you think?


Jeremias Maerki (who's glad that we're not that far apart)


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


Mime
View raw message