Le 13 avr. 04, � 21:54, Ugo Cei a �crit :
The subject of this RT is the issue of checked vs. unchecked exceptions and the horrible things the abuse of checked exceptions does to our code base...
Agreed - and I've read some of the stuff you mention, but I have a slightly different point of view: I tend to declare "throws Exception" everywhere (or at least in interface declarations), as using unchecked exceptions only can be very confusing to programmers sometimes. Using the throws clause makes it clear that you expect something to eventually happen.
Let's see how this compares with your proposal:
- use unchecked exceptions *by default* unless there's a good reason otherwise, not just because that's the way it's always been done.
I'd use Exception by default instead, to make it plain that I expect something to happen.
Are you *really* suggesting that doing:
public someMethod() throws Exception
everywhere is good programming practice?
- write tests to verify that methods don't throw exceptions for valid inputs and that they throw appropriate exceptions (e.g. InvalidArgumentException) for invalid ones.
Sounds good in an ideal world, but do you think this will happen?
In the real world, what happens is that your checked exceptions get either swallowed or rethrown as unrelated types of exceptions
Here's a case of the former:
} catch (HttpException e) {
if (getLogger().isDebugEnabled()) {
final String message =
"Unable to get WebDAV children. Server responded " +
e.getReasonCode() + " (" + e.getReason() + ") - "
+ e.getMessage();
getLogger().debug(message);
}
} catch (SAXException e) {
if (getLogger().isDebugEnabled()) {
final String message =
"Unable to get WebDAV children: "
+ e.getMessage();
getLogger().debug(message,e);
}
} catch (IOException e) {
if (getLogger().isDebugEnabled()) {
final String message =
"Unable to get WebDAV children: "
+ e.getMessage();
getLogger().debug(message,e);
}
} catch (Exception e) {
if (getLogger().isDebugEnabled()) {
final String message =
"Unable to get WebDAV children: "
+ e.getMessage();
getLogger().debug(message,e);
}
}
And here of the latter:
catch (ParserConfigurationException pce) {
throw new SAXException("Couldn't get a DocumentBuilder", pce);
}Then you have such gems as methods whose only purpose is to wrap exceptions around other exceptions in order to conform to the "throws" clause on some interface:
static public ProcessingException handle(String message,
SourceException se) {
if (se instanceof SourceNotFoundException) {
return new ResourceNotFoundException(message, se);
}
return new ProcessingException(message, se);
}In the real world, you don't have tests anyway, and programmers will make the compiler happy by swallowing exceptions and barely logging them. This is what we have now.
In the ideal world that I foresee, once in a while an unchecked exception bubbles up to the UI, users file a bug report and the bug gets fixed. While fixing the bug, a conscientious programmer writes a test to ensure that the bug is gone for good.
- document with a throws clause any unchecked exception that you explicitly throw in a method, even if it's not required to do so. Document it in the javadoc comment too....
Now that's a problem - the risk of overlooking an exception that you throw is high, whereas by using "throws Exception" you can check quickly by removing the throws clause and letting the compiler complain.
Since, *most* of the time, you have no idea what to do with the exception you've caught, you might as well overlook it. At least, you'll save a few lines of code.
With your approach, you end up with code like:
try {
x.someMethod(); // throws Exception
}
catch(Exception ignored) {
// TODO: do something with this exception
}That is, if you're lucky enough to have programmers put in a TODO comment in there. And nobody is ever going to remove the "throws" clause.
Another typical idiom is the following:
java.sql.Connection conn = null;
try {
conn = DriverManager.getConnection(...);
...
}
finally {
if (conn != null) {
try { conn.close(); } catch(SQLException ignored) {
// May the sky fall upon whomever decided that Connection.close()
// should throw an SQLException!
}
}
}Admittedly there are some cases where you might do something to recover from an exception, but what can you do when:
- parsing an XML file fails because the document is malformed?
- performing an SQL query fails because a column has been dropped?
- retrieving an Avalon component fails because the component is missing or misconfigured?
In my opinion, the best you can do is wrap the {SAX,SQL,Component}Exception in a class derived from RuntimeException and throw it, so that only the layer responsible for error handling is notified, and all the intervening layers don't have to rethrow it just because they are forced to by the compiler.
To sum it up, there is *NO* guarantee that, by using checked exceptions, you get more correct code. On the other hand, it is certain that your code will be more verbose and concerned with doing things that are not its concern, like catching and rethrowing exceptions. This just clutters a method's responsibility with extra, irrelevant instructions.
I don't have rock-solid arguments for my point of view, personal taste certainly plays a big role in such discussions. Let's hear what others think...
-Bertrand
I must say that I started this discussion with an open mind, but the more I think about it, the more I am convinced that Bruce Eckel is right: Java doesn't need checked exceptions, they are a failed experiment.
Ugo
