You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tiles.apache.org by Greg Reddin <gr...@gmail.com> on 2008/03/17 15:53:38 UTC

Re: svn commit: r637434 [1/2] - in /tiles/framework/trunk: tiles-api/src/main/java/org/apache/tiles/ tiles-api/src/main/java/org/apache/tiles/access/ tiles-api/src/main/java/org/apache/tiles/mgmt/ tiles-api/src/test/java/org/apache/tiles/access/ tile

On Sat, Mar 15, 2008 at 10:48 AM,  <ap...@apache.org> wrote:
>  Log:
>  TILES-253
>  Reorganized exceptions: now TilesException is a RuntimeException.
>  Created new specific exceptions.

I haven't had time to fully investigate this, so I apologize if I
misunderstand. But I'm not real comfortable with the idea of
completely removing checked exceptions and making everything a runtime
exception. I understand the need to support runtime exceptions and am
not opposed to creating a TilesRuntimeException or something of the
like, but it seems like maybe there are still parts of the API that
might need to declare checked exceptions.

What are your thoughts on that? And does anyone else have any thoughts about it?

Thanks,
Greg

Re: svn commit: r637434 [1/2] - in /tiles/framework/trunk: tiles-api/src/main/java/org/apache/tiles/ tiles-api/src/main/java/org/apache/tiles/access/ tiles-api/src/main/java/org/apache/tiles/mgmt/ tiles-api/src/test/java/org/apache/tiles/access/ tile

Posted by Nathan Bubna <nb...@gmail.com>.
On Mon, Mar 17, 2008 at 8:02 AM, Antonio Petrelli
<an...@gmail.com> wrote:
> 2008/3/17, Greg Reddin <gr...@gmail.com>:
>
> >
>  > On Sat, Mar 15, 2008 at 10:48 AM,  <ap...@apache.org> wrote:
>  > >  Log:
>  > >  TILES-253
>  > >  Reorganized exceptions: now TilesException is a RuntimeException.
>  > >  Created new specific exceptions.
>  >
>  > I haven't had time to fully investigate this, so I apologize if I
>  > misunderstand. But I'm not real comfortable with the idea of
>  > completely removing checked exceptions and making everything a runtime
>  > exception. I understand the need to support runtime exceptions and am
>  > not opposed to creating a TilesRuntimeException or something of the
>  > like, but it seems like maybe there are still parts of the API that
>  > might need to declare checked exceptions.
>
>
>
>  Sorry if I seem rude, but I think that the choice of using only checked
>  exceptions was a wrong choice. If you take a look on where TilesException
>  (and its extended exceptions) is thrown, you may notice that, in fact, they
>  are all configuration exceptions (errors in xml files, problems with
>  instantiations, etc.).
>  In fact I was thinking of let all the exceptions extend a
>  TilesRuntimeException, but when I noticed that all the exception were, in
>  fact, "system" exceptions, I decided to transform TilesException into a
>  RuntimeException.
>  For example, take Hibernate, where everything may throw an unchecked
>  exception: this helps a lot in finding bugs.
>  The binary compatibility will be preserved, but the readability of the code
>  is really improved.
>  But, anyway, SVN is there also for reverting changes :-)

Not having time to examine all the cases, here's my 2 cents.  If there
are any exceptions that we can reasonable expect the calling code to
handle and recover from, those should be checked.  Otherwise, we
should discourage any unnecessary/unhelpful exception catching by
switching the rest to runtime exceptions.


>  Antonio
>

Re: svn commit: r637434 [1/2] - in /tiles/framework/trunk: tiles-api/src/main/java/org/apache/tiles/ tiles-api/src/main/java/org/apache/tiles/access/ tiles-api/src/main/java/org/apache/tiles/mgmt/ tiles-api/src/test/java/org/apache/tiles/access/ tile

Posted by Antonio Petrelli <an...@gmail.com>.
2008/3/20, Greg Reddin <gr...@gmail.com>:
>
> On Mon, Mar 17, 2008 at 11:21 AM, Greg Reddin <gr...@gmail.com> wrote:
> > On Mon, Mar 17, 2008 at 10:02 AM, Antonio Petrelli
> >
>
> >  >  If you take a look on where TilesException
> >  >  (and its extended exceptions) is thrown, you may notice that, in
> fact, they
> >  >  are all configuration exceptions (errors in xml files, problems with
> >  >  instantiations, etc.).
>
>
> Just to confirm, I have studied your changes further and I no longer
> object. I am happy with the use of RuntimeException in these cases.
> Carry on :-)



Whew! At least I don't have to touch that code again, it was pretty boring
to remove "throws" declarations :-)

Antonio

Re: svn commit: r637434 [1/2] - in /tiles/framework/trunk: tiles-api/src/main/java/org/apache/tiles/ tiles-api/src/main/java/org/apache/tiles/access/ tiles-api/src/main/java/org/apache/tiles/mgmt/ tiles-api/src/test/java/org/apache/tiles/access/ tile

Posted by Greg Reddin <gr...@gmail.com>.
On Mon, Mar 17, 2008 at 11:21 AM, Greg Reddin <gr...@gmail.com> wrote:
> On Mon, Mar 17, 2008 at 10:02 AM, Antonio Petrelli
>
>  >  If you take a look on where TilesException
>  >  (and its extended exceptions) is thrown, you may notice that, in fact, they
>  >  are all configuration exceptions (errors in xml files, problems with
>  >  instantiations, etc.).

Just to confirm, I have studied your changes further and I no longer
object. I am happy with the use of RuntimeException in these cases.
Carry on :-)

Thanks,
Greg

Re: svn commit: r637434 [1/2] - in /tiles/framework/trunk: tiles-api/src/main/java/org/apache/tiles/ tiles-api/src/main/java/org/apache/tiles/access/ tiles-api/src/main/java/org/apache/tiles/mgmt/ tiles-api/src/test/java/org/apache/tiles/access/ tile

Posted by Greg Reddin <gr...@gmail.com>.
On Mon, Mar 17, 2008 at 10:02 AM, Antonio Petrelli
<an...@gmail.com> wrote:
>
>  Sorry if I seem rude, but I think that the choice of using only checked
>  exceptions was a wrong choice.

Nope, not rude at all, just a stylistic difference, and one I'm
willing to rethink.

>  If you take a look on where TilesException
>  (and its extended exceptions) is thrown, you may notice that, in fact, they
>  are all configuration exceptions (errors in xml files, problems with
>  instantiations, etc.).

So you want to see cleaner code by not having to wrap that sort of
thing in a try...catch block? As Nathan pointed out, it would likely
be a try...catch that you can't recover from. IOW, it would be a
situation where you need to see a dramatic explosion that indicates
very plainly that you have a configuration problem. But you don't want
to clutter your code by wrapping the possibility of a configuration
problem you can't do anything about in an exception handler. That
makes sense.

>  For example, take Hibernate, where everything may throw an unchecked
>  exception: this helps a lot in finding bugs.

I honestly didn't realize HibernateException was a RuntimeException
until just now. And I looked at my code which uses the Hibernate API
and all I can do is log an error in the places where I catch
HibernateException. Hmmm.

I'll take a closer look at the change in that light. If it still
bothers me I'll let you know.

Thanks,
Greg

Re: svn commit: r637434 [1/2] - in /tiles/framework/trunk: tiles-api/src/main/java/org/apache/tiles/ tiles-api/src/main/java/org/apache/tiles/access/ tiles-api/src/main/java/org/apache/tiles/mgmt/ tiles-api/src/test/java/org/apache/tiles/access/ tile

Posted by Antonio Petrelli <an...@gmail.com>.
2008/3/17, Greg Reddin <gr...@gmail.com>:
>
> On Sat, Mar 15, 2008 at 10:48 AM,  <ap...@apache.org> wrote:
> >  Log:
> >  TILES-253
> >  Reorganized exceptions: now TilesException is a RuntimeException.
> >  Created new specific exceptions.
>
> I haven't had time to fully investigate this, so I apologize if I
> misunderstand. But I'm not real comfortable with the idea of
> completely removing checked exceptions and making everything a runtime
> exception. I understand the need to support runtime exceptions and am
> not opposed to creating a TilesRuntimeException or something of the
> like, but it seems like maybe there are still parts of the API that
> might need to declare checked exceptions.



Sorry if I seem rude, but I think that the choice of using only checked
exceptions was a wrong choice. If you take a look on where TilesException
(and its extended exceptions) is thrown, you may notice that, in fact, they
are all configuration exceptions (errors in xml files, problems with
instantiations, etc.).
In fact I was thinking of let all the exceptions extend a
TilesRuntimeException, but when I noticed that all the exception were, in
fact, "system" exceptions, I decided to transform TilesException into a
RuntimeException.
For example, take Hibernate, where everything may throw an unchecked
exception: this helps a lot in finding bugs.
The binary compatibility will be preserved, but the readability of the code
is really improved.
But, anyway, SVN is there also for reverting changes :-)

Antonio