 |
Closures and language-level XML support are all good but ...
Posted by kohsuke on October 16, 2006 at 04:01 PM | Comments (19)
I just had to write this in the JAX-WS RI:
try {
return ir.getConstructor(Class.class).newInstance(clazz);
} catch (InstantiationException e) {
throw new WebServiceException(ServerMessages.FAILED_TO_INSTANTIATE_INSTANCE_RESOLVER(
ir.getName(),a.annotationType(),clazz.getName()));
} catch (IllegalAccessException e) {
throw new WebServiceException(ServerMessages.FAILED_TO_INSTANTIATE_INSTANCE_RESOLVER(
ir.getName(),a.annotationType(),clazz.getName()));
} catch (InvocationTargetException e) {
throw new WebServiceException(ServerMessages.FAILED_TO_INSTANTIATE_INSTANCE_RESOLVER(
ir.getName(),a.annotationType(),clazz.getName()));
} catch (NoSuchMethodException e) {
throw new WebServiceException(ServerMessages.FAILED_TO_INSTANTIATE_INSTANCE_RESOLVER(
ir.getName(),a.annotationType(),clazz.getName()));
}
The closures and language-level XML support are all good, but it would be really nice if the JavaSE can fix those things that we all face in day-to-day programming. For example, I think it's already filed somewhere that we should be able to write the above as the following, where e is typed as the common base class (Exception in this case):
try {
return ir.getConstructor(Class.class).newInstance(clazz);
} catch (InstantiationException,IllegalAccessException,InvocationTargetException,NoSuchMethodException e) {
throw new WebServiceException(ServerMessages.FAILED_TO_INSTANTIATE_INSTANCE_RESOLVER(
ir.getName(),a.annotationType(),clazz.getName()));
}
(Although I guess this one could argue that this particular example is a library design problem — if only we had something like ReflectionException where all the other reflection related exceptions derive.)
Bookmark blog post: del.icio.us Digg DZone Furl Reddit
Comments
Comments are listed in date ascending order (oldest first)
-
So for this particular case - why not catch Exception?
Posted by: kirillcool on October 16, 2006 at 04:04 PM
-
Because then I'd be masking things like NullPointerException, which I don't want to catch.
Posted by: kohsuke on October 16, 2006 at 04:07 PM
-
It just happens, that today I found a way to avoid this kind of duplication through some creative use of Generics. You would just write
try {
return ir.getConstructor(Class.class).newInstance(clazz);
} catch (Exception e) {
if ((e instanceof InstantiationException)
|| (e instanceof IllegalAccessException)
|| (e instanceof InvocationTargetException)
|| (e instanceof NoSuchMethodException)) {
throw new WebServiceException(ServerMessages.FAILED_TO_INSTANTIATE_INSTANCE_RESOLVER(
ir.getName(),a.annotationType(),clazz.getName()));
} else {
rethrowAny(e);
}
}
The implementation of rethrowAny(Throwable) uses Generics to circumvent Java's enforcing of checked exceptions:
/**
* Throws any throwable without the need to declare them, even if it has a
* checked exception type. It is recommended to use this only to rethrow
* intercepted exceptions of an unknown type. Example:
*
* <pre>
* try {
* someMethodThatMightFail();
* } catch (Throwable t) {
* // act on the failure
* rethrowAny(t);
* }
* </pre>
*
* <b>Note:</b> this method will never end normally.
*
* @param t
* the Throwable to throw.
*/
public static void rethrowAny(Throwable t) {
rethrowInternal(t, RuntimeException.class);
}
// Internal method, used by rethrowAny to do its magic.
@SuppressWarnings("unchecked")
private static <T extends Throwable> void rethrowInternal(Throwable t,
Class<T> c) throws T {
throw (T) t;
}
For those who can read German, here is my original Usenet posting
Enjoy!
Posted by: rullrich on October 16, 2006 at 04:31 PM
-
One way for JavaSE to address this without a language change would be to introduce a CheckedException class. Exception should just have two direct subclasses: CheckedException and RuntimeException. The common case is to want to catch all the checked exceptions, right?
Posted by: richunger on October 16, 2006 at 04:33 PM
-
rullrich — Wow. Just wow. I didn't know that you can put type variable in the "throws" clause. This is good to know, but it has some problems, as you probably know.
richunger — Yes. That'd also work for me, although the problem with that is that it'll take another few years before I can take advantage of that, because all the existing exception classes need to migrate to CheckedException. For example, say if I need to catch XMLStreamException (StAX), JAXBException (JAXB), and SAXException (SAX).
Posted by: kohsuke on October 16, 2006 at 04:45 PM
-
I think this is basically poor API design, but the fact remains that we have to deal with it. This is one of the causes of my most hated Java idiom - just catching java.lang.Exception, which treats RuntimeExceptions and checked exceptions the same way. You never want to do that. One way to deal with this is:
try
{
return ir.getConstructor(Class.class).newInstance(clazz);
}
catch (RuntimeException e)
{
throw e;
}
catch (Exception e)
{
throw new WebServiceException(ServerMessages.FAILED_TO_INSTANTIATE_INSTANCE_RESOLVER(
ir.getName(),a.annotationType(),clazz.getName()));
}
richunger is correct, we really want a CheckedException class.
Posted by: quelgar on October 16, 2006 at 05:55 PM
-
@rullrichRalf, that is a very fascinating technique, well done!Unfortunately, it gives me grave concern about the degeneration of the Java language recently.I suspect that even a few years ago, the language leads at Sun would have insisted that could never be valid Java syntax; yet here it is.I passed on J5, and with all this fuss about intentionally adding referentially opaque (i.e. poisoned) closures; I am very rapidly losing interest in J6.To me, and I am sure a lot of others, Java appears to be devolving into syntactic garbage.I supect I am not the only one who believes, where the language is currently heading; the majority of developers will not follow.John
Posted by: cajo on October 16, 2006 at 08:22 PM
-
A +1 from me on having a ReflectionException that InstantiationException,IllegalAccessException,InvocationTargetException and NoSuchMethodException can extend. File an RFE for this?
Posted by: benloud on October 16, 2006 at 09:57 PM
-
@rullrich, the language isn't degenerating, it's offering up optional tighter control (generics). I think generics are a huge win in the language where run time type safety became a serious problem. As for closures, I agree with you, skip them. Like any language, you can choose the features you want to use or not use.
Posted by: alski on October 17, 2006 at 04:18 AM
-
I filed 6482910 for ReflectionException.
Posted by: kohsuke on October 17, 2006 at 06:47 AM
-
@rullrich: What an ingenuous abuse of Generics ;-)
Note that you may get rid of the gratuitous Class parameter altogether:
public static void rethrowAny(Throwable t) {
ThisClassName.rethrowInternal(t, RuntimeException.class);
}
// Internal method, used by rethrowAny to do its magic.
@SuppressWarnings("unchecked")
private static void rethrowInternal(Throwable t) throws T {
throw (T) t;
}
Posted by: mcnepp on October 17, 2006 at 06:53 AM
-
Ooops, some tags got swallowed. I meant:
public static void rethrowAny(Throwable t) {
ThisClassName.<RuntimeException>rethrowInternal(t);
}
// Internal method, used by rethrowAny to do its magic.
@SuppressWarnings("unchecked")
private static void rethrowInternal(Throwable t) throws T {
throw (T) t;
}
Posted by: mcnepp on October 17, 2006 at 06:55 AM
-
@mcnepp: Thx for the 'improvement'. This C.<T>m() kind of syntax still eludes me. I know it exists, but every time, I try to use it, I can't remember how to do it exactly.
@cajo and @alski: I share your concerns, but not your conclusions. I think it is either Java that adapts to the wishes of the programmers or otherwise the programmers will adopt a new language, because they certainly won't adapt their wishes to match Javas capabilities.
Posted by: rullrich on October 17, 2006 at 07:26 AM
-
I think I'd write that as:
final Exception cause;
try {
return ir.getConstructor(Class.class).newInstance(clazz);
} catch (InstantiationException exc) {
cause = exc;
} catch (IllegalAccessException exc) {
cause = exc;
} catch (InvocationTargetException exc) {
cause = exc;
} catch (NoSuchMethodException exc) {
cause = exc;
}
throw new WebServiceException(
ServerMessages.FAILED_TO_INSTANTIATE_INSTANCE_RESOLVER(
ir.getName(), a.annotationType(), clazz.getName()
)
);
See, no need to modify the language for an obscure use case. OTOH, I can see uses for allowing '|' and '&' of types.
Posted by: tackline on October 17, 2006 at 07:33 AM
-
tackline — I don't think this is an obscure use case. It's an idiom that every developer faces often enough.
Posted by: kohsuke on October 17, 2006 at 08:34 AM
-
I like Kohsuke's idea best. Doing a catch-all is bad as you are catching things you shouldn't be. Kohsuke's approach is syntactic suger, to be sure, but I think it is more than justified by the improved code readability and ease of development.
Instead of e being cast to type Exception, I'd like the compiler to be smart enough to find the greatest common factor (superclass) of all types and use that instead. It could be Exception but it could potentially be something more useful.
Gili
Posted by: cowwoc on October 17, 2006 at 12:30 PM
-
Cleaning up exceptions would definately be a Godsend. If it is at all possible to do, I think richunger's idea of introducing the CheckedException class would be great.
Also, it would be nice to fix some of the Checked exceptions that should be unchecked: CloneNotSupportedException, UnsupportedEncodingException, MalformedURLException, ClassNotFoundException, and IllegalAccessException are some of the most serious offenders.
Posted by: jhannes on October 17, 2006 at 03:27 PM
-
The technique of using generics to throw checked exceptions as if they were unchecked is ingenious but obviously dangerous as well. The throws clause is part of the contract of a method. If your method does not include a particular checked exception in its throws clause then callers of the method are entitled to assume that it cannot throw that exception. In the absence of tricks, the compiler ensures that that contract is respected.
The type hole introduced by generics allows you to violate the contract. (It could already be violated by native methods.) I don't see any reason other than aesthetic not to exploit this hole for convenience within a class, but undeclared checked exceptions must not be allowed to escape from the public methods of your API.
Posted by: emcmanus on October 18, 2006 at 12:39 AM
-
rullrich: This construct is useful, but it introduces a dangerous problem.
It keeps the compiler from verifying that you are catching all of the exceptions you should be. Consider the following method (assume all of the exceptions are checked exception):
static void foo() throws AException, BException, CException { ... }
If you use your technique to handle AException, BException and CException the same way, it will work. But what if the method later changes to throw an additional exception?
static void foo() throws AException, BException, CException, DException { ... }
Your calling code should change to also handle DException. Normally the compiler could tell you that this exception isn't being properly handled. But since your technique catches Exception, this is no longer possible, and you end up with a nasty error at runtime.
Also, what if one of the exceptions is no longer thrown?
static void foo() throws AException, BException { ... }
The code that references CException is now dead, but you have to manually check to confirm this fact.
Kohsuke's excellent original suggestion doesn't have this problem; the compiler can still detect exceptions that aren't handled (as well as exceptions that are handled needlessly).
Don't get me wrong; this is absolutely annoying, and IMO the current situation could definitely be improved. But I am afraid of solutions that introduce other problems.
Posted by: seanreilly on October 18, 2006 at 12:29 PM
|