You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Morgan Delagrange <md...@yahoo.com> on 2003/01/23 21:50:14 UTC

[Jelly] Release Issue 3 - Exception handling

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.

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.

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

Steps 1 and 3 can probably be made without impacting
too many users.  Step 2, on the other hand, will
probably require changes to most of our tag
implementations in Jelly, and probably some of the
tags in Maven and Latka as well.  I'm willing to take
on as much of the Jelly and Latka work as necessary,
and I can help with the Maven tags.

I realize this is sort of a pain, but I think it's
worth doing before release.  I'd like to give it a
shot.  Comments?

- Morgan 

=====
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:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [Jelly] Release Issue 3 - Exception handling

Posted by David Kennedy <ap...@dsl.pipex.com>.
On Thursday, January 23, 2003, at 09:56  pm, Rodney Waldhoff wrote:
> While there may be cases in which "catch(Exception e)" could be construed
> as reasonable thing to do, I think we'd be hard pressed to ever justify
> "catch(Throwable t)".

+1
There are very, very few cases where you want to catch an error, and
then only in something like an appserver where choices remain.

> (I'm also in favor of weeding out "catch(Exception e)" and for that matter
> "throws Exception" wherever possible, but Throwable should absolutely go
> IMO.)

"throws Exception" is very evil.
However,
	catch(SpecificExceptionA a)
	{
	}
	catch(SpecificException b)
	{
	}
	catch(Exception e)
	{
	}
is quite justifiable. Often, the last case is involved in translating the
exception into something suitable for the error above; ideally nesting
the original. In particular, I very much dislike somelike Ant or Jelly
dying with the old "NullPointerException". I'd prefer at least to
get a specific error telling where to report it to, and apologising...

--
David Kennedy
david.kennedy@dsl.pipex.com


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [Jelly] Release Issue 3 - Exception handling

Posted by Rodney Waldhoff <rw...@apache.org>.
> Morgan Delagrange wrote:
>
> >I'd like permission to try to
> >weed out nearly all of our instances of
> >throwing/catching generic Exceptions and Throwables.
>

While there may be cases in which "catch(Exception e)" could be construed
as reasonable thing to do, I think we'd be hard pressed to ever justify
"catch(Throwable t)".  Is there every a time when when we really want to
catch the Error side of the Throwable tree (StackOverflow, OutOfMemory,
etc.)?  The most meaningful action for jelly and most code is probably to
just allow the Error to propagate up.

(I'm also in favor of weeding out "catch(Exception e)" and for that matter
"throws Exception" wherever possible, but Throwable should absolutely go
IMO.)


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [Jelly] Release Issue 3 - Exception handling

Posted by Robert McIntosh <ro...@bull-enterprises.com>.

Morgan Delagrange wrote:

>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.
>
>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.)
>
As a user, I have one comment about the above. In some cases, many 
exception types go together, like the two you mention which relate to 
class loading and getting an instance, and I have found that in cases 
like that I tend to just catch the Throwable anyway as I just need to 
know that the class couldn't be instantiated. For example in places 
where I use BeanUtils and it throws like 4 or 5 exceptions that are all 
basically the same end cause I don't want to catch all of them 
individually. In those cases I would rather have one JellyException to 
catch, which I can log/rethrow, etc.

It may not be 'correct', but if I'm using Jelly and something happens 
under the covers, like a tag not loading a class, then I want Jelly to 
tell me, not java.lang.*Exception.

>
>2) Replace the generic Exception calls in the tag
>method signatues with JellyTagException, which will be
>a subclass of JellyException.
>
I DO agree with this one and the next one. To me a framework/API should 
probably not throw Exception or Throwable on its own. Just looks like 
laziness...

Just my 2 cents..

Robert McIntosh

>
>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).
>
>Steps 1 and 3 can probably be made without impacting
>too many users.  Step 2, on the other hand, will
>probably require changes to most of our tag
>implementations in Jelly, and probably some of the
>tags in Maven and Latka as well.  I'm willing to take
>on as much of the Jelly and Latka work as necessary,
>and I can help with the Maven tags.
>
>I realize this is sort of a pain, but I think it's
>worth doing before release.  I'd like to give it a
>shot.  Comments?
>
>- Morgan 
>
>=====
>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:   <ma...@jakarta.apache.org>
>For additional commands, e-mail: <ma...@jakarta.apache.org>
>
>  
>


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [Jelly] Release Issue 3 - Exception handling

Posted by Morgan Delagrange <md...@yahoo.com>.
--- bob mcwhirter <bo...@werken.com> wrote:
> 
> I'll succumb then, as Strachan has spoken.
> 
> All hail the Strachan, and I'll lift my threat of
> veto.
> 
> Now, looking for volunteers to help migrate all of
> my taglibs... :)
> 

I volunteer.  Look for patches on this list.

- Morgan

> 
> 	-bob
> 


=====
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: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [Jelly] Release Issue 3 - Exception handling

Posted by bob mcwhirter <bo...@werken.com>.
> Now all this hard works been done by you and dIon and most of the pain has
> gone (apart from bob's) I guess in the long term this may be for the best.
> Though I'm still not quite convinced, just not prepared to veto this move as
> it does make sense in some ways.

I'll succumb then, as Strachan has spoken.

All hail the Strachan, and I'll lift my threat of veto.

Now, looking for volunteers to help migrate all of my taglibs... :)

Can I at least request that we don't replace the SNAPSHOT on ibiblio without
a large warning here on this list?  Ideally pushing a non-SNAPSHOT numbered
release that represents jelly before the change would be ideal.

	-bob


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


Re: [Jelly] Release Issue 3 - Exception handling

Posted by James Strachan <ja...@yahoo.co.uk>.
From: "Morgan Delagrange" <md...@yahoo.com>
> --- James Strachan <ja...@yahoo.co.uk> wrote:
> > 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.  :)

:)

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

Now all this hard works been done by you and dIon and most of the pain has
gone (apart from bob's) I guess in the long term this may be for the best.
Though I'm still not quite convinced, just not prepared to veto this move as
it does make sense in some ways.

James
-------
http://radio.weblogs.com/0112098/


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


Re: [Jelly] Release Issue 3 - Exception handling

Posted by Morgan Delagrange <md...@yahoo.com>.
--- bob mcwhirter <bo...@werken.com> wrote:
> 
> Having to update drools, werkz, blissed, petridish,
> and werkflow just
> isn't going to be fun.
> 

I've supplied patches for drools, blissed, and
werkflow.  I couldn't find any tags in the petridish
repository.  Werkz is already updated, probably by
dIon.  Let me know how it goes.

- Morgan

=====
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: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [Jelly] Release Issue 3 - Exception handling

Posted by di...@multitask.com.au.
Hey Bob,

bob mcwhirter <bo...@werken.com> wrote on 30/01/2003 01:32:43 AM:

[snippage]
> I apologize for my goofy example, so, lemme try again, catching what
> I want, and letting others escape:
> 
> public void doTag(XMLOutput) throws Exception
> {
>     Object obj = null;
> 
>     try
>     {
>         obj = doSomethingThatCanThrowSixException();
>     }
>     catch (FileNotFoundException e)
>     {
>         obj = new Object();
>     }
> 
>     doSomethingWithObject( obj );
> }
> 
>    ...versus...
> 
> public void doTag(XMLOutput) throws JellyException
> {
>     Object obj = null;
> 
>     try
>     {
>         obj = doSomethingThatCanThrowSixException();
>     }
>     catch (FileNotFoundException e)
>     {
>         obj = new Object();
>     }

WTF is catch Exception doing here? I'd much rather you caught all six of 
them.....

>     catch (Exception e)
>     {
>         throw new JellyException( e );
>     }
> 
>     doSomethingWithObject( obj );
> }
> 
> I guess I need to hear Strachan's opinion on the topic.
> 
> You say dIon is working on making maven happy with this? 
Yip.

> Sorry, I'm trying to stir up trouble, but that's just a radical change
> that doesn't appear to have any direct benefit to the people it affects,
> namely the tag authors.
Oh yeah, and it's a pain to fix the tags, but not too bad.
--
dIon Gillard, Multitask Consulting
Blog:      http://www.freeroller.net/page/dion/Weblog
Work:      http://www.multitask.com.au



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


Re: [Jelly] Release Issue 3 - Exception handling

Posted by bob mcwhirter <bo...@werken.com>.
> Which do you think is better, Jelly this week (no
> methods throwing generic Exceptions) or last week (all
> methods throwing Excecption).  A veto means we go back
> to the old way, where all Exceptions that Jelly throws
> are completely undocumented.  To me that's very
> undesirable.  

bare Exception in the core of jelly isn't a good thing, I'll agree with
you.  It's purely the changing of the doTag() signature that bothers me.

Though, I take it now, the two are inseparable?  You can't leave just
the doTag() siganature as throwing a bare Exception?

Having to update drools, werkz, blissed, petridish, and werkflow just
isn't going to be fun.

I apologize for my goofy example, so, lemme try again, catching what
I want, and letting others escape:

public void doTag(XMLOutput) throws Exception
{
    Object obj = null;

    try
    {
        obj = doSomethingThatCanThrowSixException();
    }
    catch (FileNotFoundException e)
    {
        obj = new Object();
    }

    doSomethingWithObject( obj );
}

	...versus...

public void doTag(XMLOutput) throws JellyException
{
    Object obj = null;

    try
    {
        obj = doSomethingThatCanThrowSixException();
    }
    catch (FileNotFoundException e)
    {
        obj = new Object();
    }
    catch (Exception e)
    {
        throw new JellyException( e );
    }

    doSomethingWithObject( obj );
}

I guess I need to hear Strachan's opinion on the topic.

You say dIon is working on making maven happy with this?  

Sorry, I'm trying to stir up trouble, but that's just a radical change
that doesn't appear to have any direct benefit to the people it affects,
namely the tag authors.

	-bob


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


Re: [Jelly] Release Issue 3 - Exception handling

Posted by Morgan Delagrange <md...@yahoo.com>.
--- bob mcwhirter <bo...@werken.com> wrote:
> 
> Golly, does this break pretty much all existing
> tags?
> (bob scans his bazillion tags...)

Is this drools you're talking about?  Yes, every tag
that throws Exception will no longer compile against
the HEAD of Jelly.  Not desirable, I agree, but also
unavoidable and specifically mentioned in the proposal
made last Thursday:

 
http://marc.theaimsgroup.com/?l=jakarta-commons-dev&m=104335501905419&w=2

> I don't really buy the style argument regarding
> making it easier
> for readers vs. authors of tags.  I don't think this
> is a place
> to assert a particular dogma.  

I think it makes a difference some of in the
non-trivial tags.

> The issue, as I see it, is about error-handling. 
> Narrowing the
> type thrown doesn't help me handle my exceptions at
> all.  Jelly doesn't
> care about the type, one way or the other.
> 
> As it stands now, you can certainly narrow the
> exceptions your tags
> throw.  I can throw or handle whatever I want,
> easily, without having
> to handle exceptions I don't wish to handle, purely
> to wrap.
> 
> While now I can do the following and handle
> MalformedURLExceptions
> if I'm able to, and ignore anything I'm unable to
> handle.
> 
> public void doTag(XMLOutput output) throws Exception
> {
>   URL url = null;
> 
>   try
>   {
>     url = new URL( whatever );
>   }  
>   catch (MalformedURLException e)
>   {
>     url = somethingElse();
>   }
> }
> 
> Under the new code I'd have to do this:
> 
> public void doTag(XMLOutput output) throws Exception
> {
>   URL url = null;
> 
>   try
>   {
>     url = new URL( whatever );
>   }  
>   catch (MalformedURLException e)
>   {
>     url = somethingElse();
>   }
>   catch (Exception e)
>   {
>     // a suspicious amount of handling for an
>     // unhandled exception...
>     throw new JellyException( e );
>   }
> }

I'm not sure I understand your example.  URL only
throws one type of exception (MalformedURLException),
which you are catching.  What is the purpose of the
catch(Exception) block?  If your perception is that
you need to introduce catch(Exception) blocks into all
your methods for some reason, or that somehow Jelly is
no longer equipped to handler RuntimeExceptions,
that's not the case. 

I realize these changes create another requirement for
other taglib authors.  I have to weigh this against my
belief that Jelly's internal implementations need
compile-time verification that exceptions are
accounted for and handled appropriately.

> That's not pleasing.  Arguing stylistically, I'd say
> it even forces
> me to inject noise or red-herrings into my code,
> possibly confusing the
> reader.  "Hey, he's catching and handling all
> exceptions" instead of 
> "Hey, he makes no claims about the unhandled
> exceptions escaping
> from this method."

I think users understand what try/catch blocks mean. 
It's only confusing if you aren't picky about using
them and scope whole method bodies with huge blocks. 
Yes, this means you have to handle exceptions.  I
don't want to force you to adopt a particular style,
but I think the alternative leads to a bunch of
undocumented exceptions in Jelly, and that I don't
care for.  It's tough to have it both ways. 

I think the redesign is a short term chore with long
term benefits.  If you truly view the old design as
superior, however, it's probably simple to emulate in
your own projects:

public abstract class MyTagSupport
extends TagSupport {

  public void doTag(XMLOutput output) 
  throws JellyTagException {
    try {
      doMyTag(output);
    } catch (Exception e) {
      throw new JellyTagException(e);
    }
  }

  public abstract void doMyTag(XMLOutput output)
  throws Exception;

}

> If we had an official vote on this, I'd be close to
> -1, hate to say.

Which do you think is better, Jelly this week (no
methods throwing generic Exceptions) or last week (all
methods throwing Excecption).  A veto means we go back
to the old way, where all Exceptions that Jelly throws
are completely undocumented.  To me that's very
undesirable.  

- Morgan

> 	-bob
> 
> 
> 
> On Tue, 28 Jan 2003, Morgan Delagrange wrote:
> 
> > ...doTag() signature change stuff...
> 
> 
> 
>
---------------------------------------------------------------------
> To unsubscribe, e-mail:
> commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail:
> commons-dev-help@jakarta.apache.org
> 


=====
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: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [Jelly] Release Issue 3 - Exception handling

Posted by bob mcwhirter <bo...@werken.com>.
Golly, does this break pretty much all existing tags?
(bob scans his bazillion tags...)

I don't really buy the style argument regarding making it easier
for readers vs. authors of tags.  I don't think this is a place
to assert a particular dogma.  

The issue, as I see it, is about error-handling.  Narrowing the
type thrown doesn't help me handle my exceptions at all.  Jelly doesn't
care about the type, one way or the other.

As it stands now, you can certainly narrow the exceptions your tags
throw.  I can throw or handle whatever I want, easily, without having
to handle exceptions I don't wish to handle, purely to wrap.

While now I can do the following and handle MalformedURLExceptions
if I'm able to, and ignore anything I'm unable to handle.

public void doTag(XMLOutput output) throws Exception
{
  URL url = null;

  try
  {
    url = new URL( whatever );
  }  
  catch (MalformedURLException e)
  {
    url = somethingElse();
  }
}

Under the new code I'd have to do this:

public void doTag(XMLOutput output) throws Exception
{
  URL url = null;

  try
  {
    url = new URL( whatever );
  }  
  catch (MalformedURLException e)
  {
    url = somethingElse();
  }
  catch (Exception e)
  {
    // a suspicious amount of handling for an
    // unhandled exception...
    throw new JellyException( e );
  }
}

That's not pleasing.  Arguing stylistically, I'd say it even forces
me to inject noise or red-herrings into my code, possibly confusing the
reader.  "Hey, he's catching and handling all exceptions" instead of 
"Hey, he makes no claims about the unhandled exceptions escaping
from this method."

If we had an official vote on this, I'd be close to -1, hate to say.

	-bob



On Tue, 28 Jan 2003, Morgan Delagrange wrote:

> ...doTag() signature change stuff...



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


Re: [Jelly] Release Issue 3 - Exception handling

Posted by Morgan Delagrange <md...@yahoo.com>.
--- James Strachan <ja...@yahoo.co.uk> 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" <md...@yahoo.com>
> > 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: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [Jelly] Release Issue 3 - Exception handling

Posted by di...@multitask.com.au.
James,

I have a buildable version of Maven on my hard disk waiting for the jelly 
snapshots to get uploaded to ibiblio.

I've also uploaded werkz-1.0-beta6 to ibiblio that uses the new api.
--
dIon Gillard, Multitask Consulting
Blog:      http://www.freeroller.net/page/dion/Weblog
Work:      http://www.multitask.com.au


"James Strachan" <ja...@yahoo.co.uk> wrote on 29/01/2003 12:32:44 
AM:

> 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" <md...@yahoo.com>
> > 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. 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.
> 
> 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.
> 
> Most importantly I didn't want to break all the tag implementations out
> there in Maven, drools, werkflow and alsorts of other places.
> 
> 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).
> 
> 
> > 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.
> 
> James
> -------
> http://radio.weblogs.com/0112098/
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
> 

> ForwardSourceID:NT000A9AA6 

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


Re: [Jelly] Release Issue 3 - Exception handling

Posted by James Strachan <ja...@yahoo.co.uk>.
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" <md...@yahoo.com>
> 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. 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.

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.

Most importantly I didn't want to break all the tag implementations out
there in Maven, drools, werkflow and alsorts of other places.

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


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

James
-------
http://radio.weblogs.com/0112098/


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


Re: [Jelly] Release Issue 3 - Exception handling

Posted by di...@multitask.com.au.
I'm+0 and with a little more time on my hands would be +1.
--
dIon Gillard, Multitask Consulting
Blog:      http://www.freeroller.net/page/dion/Weblog
Work:      http://www.multitask.com.au


Morgan Delagrange <md...@yahoo.com> wrote on 24/01/2003 07:50:14 AM:

> 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.
> 
> 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.
> 
> 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).
> 
> Steps 1 and 3 can probably be made without impacting
> too many users.  Step 2, on the other hand, will
> probably require changes to most of our tag
> implementations in Jelly, and probably some of the
> tags in Maven and Latka as well.  I'm willing to take
> on as much of the Jelly and Latka work as necessary,
> and I can help with the Maven tags.
> 
> I realize this is sort of a pain, but I think it's
> worth doing before release.  I'd like to give it a
> shot.  Comments?
> 
> - Morgan 
> 
> =====
> 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: 
<ma...@jakarta.apache.org>
> For additional commands, e-mail: 
<ma...@jakarta.apache.org>
> 

> ForwardSourceID:NT000A7066 

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>