You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pivot.apache.org by Noel Grandin <no...@gmail.com> on 2009/08/06 10:42:23 UTC
style of ignoring exceptions
Hi
I notice that in places Pivot has code like this:
try {
... stuff....
} catch(IllegalAccessException exception) {
} catch(InstantiationException exception) {
}
My preferred pattern for ignored exceptions is this:
try {
... stuff....
} catch(IllegalAccessException exception) {
throw new RuntimeException(exception); // should never happen
} catch(InstantiationException exception) {
throw new RuntimeException(exception); // should never happen
}
Just so that I get a stacktrace in those rare cases, where yes, somebody
did something extra-ordinarily stupid and managed to trigger the exception.
Comments?
-- Noel.
Re: style of ignoring exceptions
Posted by Greg Brown <gk...@mac.com>.
> So now, what can we do:
> create base classes for our exceptions, and maybe also one or more
> internal packages ?
No - I was referring to the changes Noel made for PIVOT-211. I don't
think we need any special-purpose exception classes.
Re: style of ignoring exceptions
Posted by Sandro Martini <sa...@gmail.com>.
Ok, thanks ...
Bye
Re: style of ignoring exceptions
Posted by Noel Grandin <no...@gmail.com>.
Sandro, Greg was replying to the original suggestion, for which I've
already checked in a fix. (PIVOT-211)
I agree with Christopher here - creating a new exception class is
unnecessary.
Regards, Noel.
Sandro Martini wrote:
> Hi Greg,
>
>> It is a good suggestion. In cases where we actually do want to ignore the
>> exception, the existing approach is obviously fine, but I can definitely see
>> the argument for using a more defensive style elsewhere.
>>
>
> So now, what can we do:
> create base classes for our exceptions, and maybe also one or more
> internal packages ?
>
> A JIRA ticket could clarify all i think ...
>
> Bye
>
Re: style of ignoring exceptions
Posted by Sandro Martini <sa...@gmail.com>.
Hi Greg,
> It is a good suggestion. In cases where we actually do want to ignore the
> exception, the existing approach is obviously fine, but I can definitely see
> the argument for using a more defensive style elsewhere.
So now, what can we do:
create base classes for our exceptions, and maybe also one or more
internal packages ?
A JIRA ticket could clarify all i think ...
Bye
Re: style of ignoring exceptions
Posted by Greg Brown <gk...@mac.com>.
It is a good suggestion. In cases where we actually do want to ignore
the exception, the existing approach is obviously fine, but I can
definitely see the argument for using a more defensive style elsewhere.
On Aug 6, 2009, at 4:42 AM, Noel Grandin wrote:
> Hi
>
> I notice that in places Pivot has code like this:
> try {
> ... stuff....
> } catch(IllegalAccessException exception) {
> } catch(InstantiationException exception) {
> }
>
> My preferred pattern for ignored exceptions is this:
> try {
> ... stuff....
> } catch(IllegalAccessException exception) {
> throw new RuntimeException(exception); // should never
> happen
> } catch(InstantiationException exception) {
> throw new RuntimeException(exception); // should never
> happen
> }
>
> Just so that I get a stacktrace in those rare cases, where yes,
> somebody
> did something extra-ordinarily stupid and managed to trigger the
> exception.
>
> Comments?
>
> -- Noel.
Re: style of ignoring exceptions
Posted by Greg Brown <gk...@mac.com>.
> I think I mentioned it before, but I still think some kind of 'top
> level
> exception handler' would still be useful. People can then plug in
> custom
> error reporting mechanisms.
We added this a couple weeks back. See
Application.UncaughtExceptionHandler.
Re: style of ignoring exceptions
Posted by Christopher Brind <br...@brindy.org.uk>.
Hi Noel,
Yes, I tend to do that as well so I'm +1 for this approach. :)
Throwing a RuntimeException is preferable to doing nothing because if those
'should never happen' exceptions do occur for whatever bizarre reason and we
just swallow them we risk putting the client in to an 'unknown state'. For
instance, I could easily imagine the code in your first example quickly
resulting in a NullPointerException further along in the execution and that
would be hard to debug. The RuntimeException removes that ambiguity.
I think I mentioned it before, but I still think some kind of 'top level
exception handler' would still be useful. People can then plug in custom
error reporting mechanisms.
Cheers,
Chris
2009/8/6 Noel Grandin <no...@gmail.com>
> Hi
>
> I notice that in places Pivot has code like this:
> try {
> ... stuff....
> } catch(IllegalAccessException exception) {
> } catch(InstantiationException exception) {
> }
>
> My preferred pattern for ignored exceptions is this:
> try {
> ... stuff....
> } catch(IllegalAccessException exception) {
> throw new RuntimeException(exception); // should never happen
> } catch(InstantiationException exception) {
> throw new RuntimeException(exception); // should never happen
> }
>
> Just so that I get a stacktrace in those rare cases, where yes, somebody
> did something extra-ordinarily stupid and managed to trigger the exception.
>
> Comments?
>
> -- Noel.
>
Re: style of ignoring exceptions
Posted by Greg Brown <gk...@mac.com>.
>> I was thinking more in the line that the exceptions are caught during
>> development and QA of the Pivot application. If it would be possible
>> to do "Install Pivot Reporting Handler" in the "dev/qa mode" of the
>> app, then when QA finds the problem, they effectively just hit the
>> 'report' button and it ends here somehow. People who has worked with
>> IntelliJ's IDEA know what I mean.
> Yes, this would be great, but i don't think it's a simple to implement
> and integrate, so why not for the 2.0 ?
I actually think this type of feature should be handled at the
application level, not in the framework. An application that wants to
automatically send bug reports could do so by implementing
Application.UncaughtExceptionHandler.
Re: style of ignoring exceptions
Posted by Sandro Martini <sa...@gmail.com>.
Hi,
I like the idea of a Pivot custom RuntimeException , and maybe also
another extending Exception ...
In other projects I've seen some internal packages (like
org.apache.tapestry.internal, but there are others), for example
Tapestry has this, and in the doc is written that all classes in
internal packages should not be used outside the project source code.
> I was thinking more in the line that the exceptions are caught during
> development and QA of the Pivot application. If it would be possible
> to do "Install Pivot Reporting Handler" in the "dev/qa mode" of the
> app, then when QA finds the problem, they effectively just hit the
> 'report' button and it ends here somehow. People who has worked with
> IntelliJ's IDEA know what I mean.
Yes, this would be great, but i don't think it's a simple to implement
and integrate, so why not for the 2.0 ?
Bye
Re: style of ignoring exceptions
Posted by Niclas Hedhman <ni...@hedhman.org>.
On Thu, Aug 6, 2009 at 5:31 PM, Christopher Brind<br...@brindy.org.uk> wrote:
> It might appear that the advantage of sub classing RuntimeException would be
> that we are able to quickly identify exceptions generated in the Pivot
> framework, but this is countered by the fact we have no way to stop
> application developers using Pivot also throwing that exception.
> Assuming the 'End User' is even able to get their hands on the exception
> stack trace, the error reporting flow is likely to be something like:
>
> End User -> (1st or 2nd line support maybe) -> App Developer -> Pivot Team
>
> The App Developer is likely to be able to determine if the problem should be
> reported to the Pivot team by simply looking at the stack trace.
I was thinking more in the line that the exceptions are caught during
development and QA of the Pivot application. If it would be possible
to do "Install Pivot Reporting Handler" in the "dev/qa mode" of the
app, then when QA finds the problem, they effectively just hit the
'report' button and it ends here somehow. People who has worked with
IntelliJ's IDEA know what I mean.
I might say that a "InternalPivotError extends Error" might discourage
enought for others to use, especially if it contains compulsory
constructor arguments like "Component"...
Cheers
--
Niclas Hedhman, Software Developer
http://www.qi4j.org - New Energy for Java
I live here; http://tinyurl.com/2qq9er
I work here; http://tinyurl.com/2ymelc
I relax here; http://tinyurl.com/2cgsug
Re: style of ignoring exceptions
Posted by Christopher Brind <br...@brindy.org.uk>.
Seems a bit unnecessary to me.
It might appear that the advantage of sub classing RuntimeException would be
that we are able to quickly identify exceptions generated in the Pivot
framework, but this is countered by the fact we have no way to stop
application developers using Pivot also throwing that exception. I suppose
we could document it like so:
"This exception is intended to be thrown by the Pivot framework. Please
> don't use this exception."
>
but my view on that is, why have that class in the API if people aren't
supposed to use it?
Assuming the 'End User' is even able to get their hands on the exception
stack trace, the error reporting flow is likely to be something like:
End User -> (1st or 2nd line support maybe) -> App Developer -> Pivot Team
The App Developer is likely to be able to determine if the problem should be
reported to the Pivot team by simply looking at the stack trace.
Cheers,
Chris
2009/8/6 Niclas Hedhman <ni...@hedhman.org>
> I would like to agree with that, BUT have a separate subclass of
> RuntimeException that is caught high up and can be used to generate a
> Report back to the Pivot team.
>
>
> -- Niclas
>
> On Thu, Aug 6, 2009 at 4:42 PM, Noel Grandin<no...@gmail.com> wrote:
> > Hi
> >
> > I notice that in places Pivot has code like this:
> > try {
> > ... stuff....
> > } catch(IllegalAccessException exception) {
> > } catch(InstantiationException exception) {
> > }
> >
> > My preferred pattern for ignored exceptions is this:
> > try {
> > ... stuff....
> > } catch(IllegalAccessException exception) {
> > throw new RuntimeException(exception); // should never happen
> > } catch(InstantiationException exception) {
> > throw new RuntimeException(exception); // should never happen
> > }
> >
> > Just so that I get a stacktrace in those rare cases, where yes, somebody
> > did something extra-ordinarily stupid and managed to trigger the
> exception.
> >
> > Comments?
> >
> > -- Noel.
> >
>
>
>
> --
> Niclas Hedhman, Software Developer
> http://www.qi4j.org - New Energy for Java
>
> I live here; http://tinyurl.com/2qq9er
> I work here; http://tinyurl.com/2ymelc
> I relax here; http://tinyurl.com/2cgsug
>
Re: style of ignoring exceptions
Posted by Niclas Hedhman <ni...@hedhman.org>.
I would like to agree with that, BUT have a separate subclass of
RuntimeException that is caught high up and can be used to generate a
Report back to the Pivot team.
-- Niclas
On Thu, Aug 6, 2009 at 4:42 PM, Noel Grandin<no...@gmail.com> wrote:
> Hi
>
> I notice that in places Pivot has code like this:
> try {
> ... stuff....
> } catch(IllegalAccessException exception) {
> } catch(InstantiationException exception) {
> }
>
> My preferred pattern for ignored exceptions is this:
> try {
> ... stuff....
> } catch(IllegalAccessException exception) {
> throw new RuntimeException(exception); // should never happen
> } catch(InstantiationException exception) {
> throw new RuntimeException(exception); // should never happen
> }
>
> Just so that I get a stacktrace in those rare cases, where yes, somebody
> did something extra-ordinarily stupid and managed to trigger the exception.
>
> Comments?
>
> -- Noel.
>
--
Niclas Hedhman, Software Developer
http://www.qi4j.org - New Energy for Java
I live here; http://tinyurl.com/2qq9er
I work here; http://tinyurl.com/2ymelc
I relax here; http://tinyurl.com/2cgsug