--- James Strachan <[EMAIL PROTECTED]> wrote:
> I guess I'm too late to argue this one any more but
> here's my 2 pennies for
> what its worth.
>
> From: "Morgan Delagrange" <[EMAIL PROTECTED]>
> > Hi all,
> >
> > I'm going to talk about Jelly Exception
> > handling...again. :) I'd like permission to try
> to
> > weed out nearly all of our instances of
> > throwing/catching generic Exceptions and
> Throwables.
> > IMO explicit error handling is better than just
> > throwing Exception. It makes the possible
> problems in
> > your application much more clear. It makes it
> easier
> > to distinguish between recoverable and
> unrecoverable
> > error. It makes it safer to change the
> application,
> > because you're aware when you introduce new types
> of
> > Exceptions to your application. It's just a good
> > thing.
>
> Thats certainly true in general.
>
>
> > I'd like to try the following steps:
> >
> > 1) Replace the generic Exception calls from all
> method
> > signatures EXCEPT TAGS with JellyException. In
> some
> > cases, there may be a more appropriate exception
> than
> > JellyException (e.g. InstantiationException,
> > ClassNotFoundException, etc.)
> >
> > 2) Replace the generic Exception calls in the tag
> > method signatues with JellyTagException, which
> will be
> > a subclass of JellyException.
>
> I'm in total agreement in general - however I
> disagree about the Tag
> interface change. The only class which will ever be
> invoking the Tag.doTag()
> method is the TagScript class.
Not strictly true (e.g. AntTag), but close to it. The
only reason I know that's not always the case is
because I happen to have read just about every Jelly
source file this week. :)
> So there's only 1
> piece of code typically
> that this method is invoked. The reason I made the
> decision to go with
> throws Exception was so that Tag developers don't
> have to bother catching
> whatever checked exceptions get thrown when they do
> their stuff and then
> wrapping them in some other exception. The TagScript
> does that automatically
> in one place. I agree that this is not standard OO
> practice but then Tags
> were not meant to be standard OO objects
> interspersed around your
> application code - rather standard OO code would
> invoke Jelly Scripts via
> the JellyContext or Script interfaces.
>
> Pretty much any tag of any complexity is gonna do
> something which may thrown
> an exception. Forcing all tags to catch all these
> exceptions and wrap them
> in some wrapper exception didn't seem to add much
> value and seems to add
> extra work to the tag author for little gain IMHO.
> Pretty much all APIs do
> this (JSP, Ant, Servlets etc). Though in something
> like Jelly it just didn't
> feel right - it seems unnecessary work for the Tag
> authors.
I agree that there is a case to be made for the
doTag() method throwing Exception. However after
working with the code, I'm fairly sure that narrowing
the doTag() exception was the right choice. Let's see
if I can assuage your doubts at least a bit. Read on,
ignore, whatever you like. Veto if you feel strongly
about it (I tagged a JELLY_PRE_EXCEPTION_REFACTOR
version prior to digging in). However I'd obviously
prefer that you didn't. :)
I took great care to make sure that this was
meaningful helpful work, not a sloppy pro forma
conversion. I avoided simply wrapping entire method
bodies inside try/catch blocks. Basically, I tried to
understand what each method was doing and make the
syntax changes that made sense, even moving statements
when necessary. For example:
Call call = (Call) service.createCall();
call.setTargetEndpointAddress(new
java.net.URL(endpoint));
call.setOperationName(new QName(namespace, method));
Object answer = call.invoke(params);
became:
Object answer = null;
try {
Call call = (Call) service.createCall();
call.setTargetEndpointAddress(new
java.net.URL(endpoint));
call.setOperationName(new QName(namespace,
method));
answer = call.invoke(params);
} catch (MalformedURLException e) { ...
Do you agree that the latter is more readable? It
makes it immediately apparent to the reader that,
beyond this block, the only important code is the
answer Object; the Call object and all its methods are
just a means to that end. While it may seem like
busywork, try/catch blocks can also add more structure
to the method body.
In a sense, making doTag() throw Exception may be more
convenient for the AUTHOR, but I'm not sure it's doing
the reader any favors.
> Also from an efficiency point of view, the TagScript
> needs to catch
> exceptions anyway, to append the location
> information into the wrapped
> JellyException - so it seems silly to have a double
> catch block.
It's not as bad as it sounds, I think. It's true that
there is an extra catch block for many execution paths
now, but the number of exception objects generated is
roughly the same. In the past, if the Tag script
received a JellyException (which it usually did not),
it would simply decorate it and rethrow it rather than
generating a new one. Now that virtually all
exceptions are JellyExceptions of one type or another,
we've simply moved the place that those exceptions are
generated.
There are sometimes some minor benefits to generating
that message at the tag level too, because you can
provide more context-specific information about the
error. If you get into the habit of tossing all
exceptions up the stack, you don't pay quite as much
attention to what they say.
> Most importantly I didn't want to break all the tag
> implementations out
> there in Maven, drools, werkflow and alsorts of
> other places.
Yeah that was the BIG bummer. However the only way to
avoid that completely was to leave all the Tag
signatures unchanged, and I think we're in agreement
that at least most of those methods should be more
specific in their exception handling. That doesn't
change the fact that doTag() is the big one though;
that's why I've raised this so many times and mulled
over it before actually making a proposal. I had to
be sure it was worth it, but I think it is. Better
now than after a 1.0 release.
Aside from the Exceptions they throw, method
signatures are virtually unchanged. Adopting should
not be too painful if it is done in a timely fashion.
I was able to perform the bulk of the conversion for
all taglib builds over a weekend.
> So I was gonna vote -1 mostly of the latter reason.
> I'm not so concerned
> about the other reasons that I'd try to veto this
> move. Though since I'm
> quite late to the party (had a very busy week) and
> most of the works been
> done already I'm -0 on this. Though please can
> someone help out fixing the
> other libraries - particularly the Maven ones. Right
> now I can't build Maven
> or Jelly as both are busted (I can't build Maven as
> its Jelly tags are
> busted, so I can't get a recent build of Maven to
> build Jelly).
The Ant scripts can still build Jelly fine, and the
GUMP builds are working. I think dIon's about ready
with a fix to Maven. I did my best to make sure every
commit resulted in a stable, end-to-end build Ant
build. However, since so much refactoring was
required and I do not have access to the ibiblio
repository, I couldn't do as much to ensure ongoing
Maven compatibility. But I did most of the work over
the weekend, so as to cause the least impact possible.
>
> > 3) Weed out most of the try/catch blocks in the
> bodies
> > of our methods that catch Exception or Throwable.
> I
> > will replace them with the specific Exceptions
> they
> > catch (probably JellyException in most cases).
>
> Agreed - catching Throwable is bad. Also
> JellyContext and Script interfaces
> should not throw Exception. I'm agreeing with you in
> these moves, just
> agreeing to disagree about the Tag interface change
> - but am not disagreeing
> that strongly - more explaining why I made the
> choices I did.
I think if you wanted to do it right, converting
everything but the doTag method would have been a much
more difficult conversion. There are tons of utility
methods inside the various Tag implementations, and
essentially all of them threw Exception. To get them
all throwing specific exceptions while still allowing
doTag() to throw Exception would have been a difficult
conversion to complete (no help from the compiler) and
impossible to ensure completely. Please don't make me
go back there. ;)
I hope that as you look at the results of the
refactoring, you'll start to see at least some
tangible benefits [crossing fingers]. I hope at the
very least, you'll agree that Jelly is better off now
than seven days ago and forgive me my eccentricities
with doTag.
- Morgan
> James
> -------
> http://radio.weblogs.com/0112098/
>
=====
Morgan Delagrange
http://jakarta.apache.org/taglibs
http://jakarta.apache.org/commons
http://axion.tigris.org
http://jakarta.apache.org/watchdog
__________________________________________________
Do you Yahoo!?
Yahoo! Mail Plus - Powerful. Affordable. Sign up now.
http://mailplus.yahoo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
