They seem to miss "Improper Exception usage", where an unchecked exception somewhere make another portion of code not executed because that exception breaks the control flow.
(See Disclaimer at the end of this post: what follows is not "against" exceptions, it is just a reminder that exceptions can be tricky ;) )
Joel has viewed Exception as "Goto in disguise" (October 2003), and explained in his "Making Wrong Code Look Wrong" (may 2005) that:
In order to make code really, really robust, when you code-review it, you need to have coding conventions that allow collocation.
In other words, the more information about what code is doing is located right in front of your eyes, the better a job you’ll do at finding the mistakes.
When you have code that says
dosomething();
cleanup();
… your eyes tell you, what’s wrong with that? We always clean up!
But the possibility that dosomething might throw an exception means that cleanup might not get called. And that’s easily fixable, using finally or whatnot, but that’s not my point:
my point is that the only way to know that cleanup is definitely called is to investigate the entire call tree of dosomething to see if there’s anything in there, anywhere, which can throw an exception, and that’s ok, and there are things like checked exceptions to make it less painful, but the real point is that exceptions eliminate collocation.
You have to look somewhere else to answer a question of whether code is doing the right thing, so you’re not able to take advantage of your eye’s built-in ability to learn to see wrong code, because there’s nothing to see.
Disclaimer: this "programming error" I mention here is not "for" or "against" exceptions (I fully agree with using Exception, and I also use "error code" when needed).
It merely points out that writing good exception-based code is hard.
As Raymond Chen pointed out:
when I write code that is exception-based, I do not have the luxury of writing bad code first and then making it not-bad later. If I did that, I wouldn't be able to find the bad code again, since it looks almost identical to not-bad code.
My point isn't that exceptions are bad. My point is that exceptions are too hard and I'm not smart enough to handle them
I am puzzled by the comments which imply that Joel's arguments are borderline "specious" (i.e. "apparently good or right though lacking real merit"):
Greg Beech: "always use finally for clean up, and it's probably right."
Richard Corden: "even if you don't explicitly throw an exception yourself - you might get an exception you don't expect (eg. allocating memory)"
Andreas Huber: "Even if dosomething() does not throw any exception today it might well tomorrow, so why not make things robust and add the finally in any case???? P.S. A little Google research would show you that pretty much everything Joel mentions w.r.t. exceptions has long been debunked"
Andreas cite this article "Lessons Learned from Specifying Exception-Safety for the C++ Standard Library" as a first example of rebuttal.
I fail to see how those comments contradict in any way:
- Joel's point: exceptions eliminate collocation
- Raymond's point: exceptions are too hard and I'm not smart enough to handle them.
For one, there are many ways to handle exceptions improperly: look at those "Exception Horror Stories" which, in themselves, should grand "Exception Management" a nice spot in that "most dangerous programming mistakes" list.
And then, there is the issue of "just using finally". "Just"... Right. As if it was simple or trivial, as if the result was still readable.
Consider this "Good housekeeping practices" article from IBM.
What is a readable code (below) is a bad, unsafe one, because exceptions are thrown:
public void enumerateFoo() throws SQLException {
Connection connection = getConnection();
Statement statement = connection.createStatement();
ResultSet resultSet = statement.executeQuery("SELECT * FROM Foo");
// Use resultSet
resultSet.close();
statement.close();
connection.close();
}
"Just add a finally" does not solve anything, while adding complexity:
public void enumerateFoo() throws SQLException {
Statement statement = null;
ResultSet resultSet = null;
Connection connection = getConnection();
try {
statement = connection.createStatement();
resultSet = statement.executeQuery("SELECT * FROM Foo");
// Use resultSet
}
finally {
if (resultSet != null) {
resultSet.close(); }
if (statement != null) {
statement.close(); }
connection.close();
}
}
Using finally to release resources acquired in a method is reliable but can easily get unwieldy when multiple resources are involved.
The reason this "solution" doesn't work is that the close() methods of ResultSet and Statement can themselves throw SQLException, which could cause the later close() statements in the finally block not to execute.
That leaves you with several choices, all of which are annoying:
- wrap each close() with a try..catch block,
- nest the try...finally blocks
- or write some sort of mini-framework for managing the resource acquisition and release.
The "nesting solution" would be:
public void enumerateBar() throws SQLException {
Statement statement = null;
ResultSet resultSet = null;
Connection connection = getConnection();
try {
statement = connection.createStatement();
resultSet = statement.executeQuery("SELECT * FROM Bar");
// Use resultSet
}
finally {
try {
if (resultSet != null) {
resultSet.close(); }
}
finally {
try {
if (statement != null) {
statement.close(); }
}
finally {
connection.close();
}
}
}
}
Aaargh... (and I do that in my code...).
Compare the above with the initial lines:
[...]
ResultSet resultSet = statement.executeQuery("SELECT * FROM Foo");
// Use resultSet
resultSet.close();
[...]
Do you see now how "exceptions eliminate collocation" ?
This business of making wrong code look wrong depends on getting the right things close together in one place on the screen: [...] get the relevant information about what a line of code really does physically as close together as possible. This improves the chances that your eyeballs will be able to figure out everything that’s going on.
With all those "null checks" and "nested finally", keeping code together is kind of hard...