To duck or not to duck

About a week ago, I gave a lunch-and-learn talk at my office on Robert C Martin’s Clean Code—it’s considered “highly suggested” reading in the office, so, as I do when that happens, I read the thing cover to cover. I’m inclined to believe that most, if not all, of the other developers on staff who had been hired by this point did as well, but one of the topics that Martin brings up in his chapter on error handling is whether or not developers should be using checked or unchecked exceptions. Martin comes down on the side of preferring unchecked; there’s no good reason, he says, to use checked exceptions that you have to keep declaring every layer up until you actually catch it—it violates the Open/Closed Principle, and it creates leaky encapsulation.

He’s got a really good point. Having to constantly redeclare your exceptions (or, God forbid, catch and rethrow) is a pain in the ass, unnecessarily exposes your code’s structure, and makes it really difficult to refactor changes to your exceptions (if, of course, you don’t use a magical IDE like IntelliJ IDEA). So there’s certainly some benefit to be gained from preferring unchecked exceptions, both in purely internal code and if you intend to publish an API.

Ultimately, though, I disagree. Unchecked exceptions, to me, should really only be used to handle completely avoidable situations; this is why the stock set of unchecked exceptions in Java includes things like NullPointerException, IllegalArgumentException, and ArrayIndexOutOfBoundsException. An unchecked exception is something that the programmer could, and should, have prevented through better care. As an aside, this is also why I feel ripped off that java.net.URLEncoder.encode() throws a checked exception—the likelihood that the encoding is being specified by anyone other than the programmer is practically zero, so making me catch an UnsupportedEncodingException that will never be thrown is ridiculous:

import static java.net.URLEncoder.encode; ... try { // "UTF-8" must be supported by the JVM, and what are you doing using any other encoding anyway? String encoded = encode("q=hey guys, what's up?", "UTF-8"); } catch (UnsupportedEncodingException e) { System.exit(); // CANTHAPPEN } ...

I also don’t believe that simply switching from extends Exception to extends RuntimeException does anything meaningful in terms of application structure when the catching method is four or five stack frames below the thrower, beyond preventing throws MyException from showing up in your method signatures. If we’re really adhering to the Law of Demeter, writing software on the assumption that a method should only know about the methods of its own variables, of its own class, and of its class’ fields, then most exceptions have no earthly business falling down the stack, particularly across class boundaries. If a method, m, throws an exception, e, then e should be an exceptional circumstance that arose while m was performing its task, not one that arose while m was waiting for a called method in another class to return. I’m willing to grant leniency to private methods, treating the class as a whole with respect to error handling, but simply put, classes should not duck exceptions.

Here’s a facile example:

package facileexamples; public class SomeController { @Inject SomeService service; public void doWork() { try { List<Entity> entities = service.getEntities(); doSomethingWith(entities); } catch (DatabaseConnectionException e) { dealWithTheMissingDatabase(); } } } package facileexamples; @Named public class SomeService { @Inject SomeDao dao; public List<Entity> getEntities() throws DatabaseConnectionException { try { return dao.getEntities(); } catch (NoResultException e) { return Collections.EMPTY_LIST; } } }

Clearly, the class designer is a jerk, who deals with NoResultException in the Service but not DatabaseConnectionException.

package facileexamples; import javax.persistence.*; @Named public class SomeDao { @Inject EntityManager entityManager; public List<Entity> getEntities() throws DatabaseConnectionException { TypedQuery<Entity> query = entityManager.createNamedQuery("entity.getEntities"); return query.getResultList(); } }

SomeService knows that there’s a possibility that the call to SomeDao might get rejected if the database connection has failed, because SomeDao declared that this might happen (this is why this is a facile example; any persistence layer worth its salt wouldn’t even throw that), but the getEntities() method is the wrong place to deal with that—it’s really just supposed to be focussed on handling Entitys, not errors. So it says one of two things:

1. I can’t deal with this; somebody above me will, or
2. this is crazy enough that the JVM should be able to unceremoniously terminate.

Neither of these options is particularly good. In the second case, application’s users don’t get meaningful error (unless they’re developers), and in the first case, a layer too-far-removed from the database has to deal with the database’s problems. SomeController shouldn’t know that there’s a database, or any kind of socket connection involved at all, when it asks SomeService to getEntities. All SomeController ought to know is that it can call a service that provides a List of Entity instances. That’s it; if something goes wrong in retrieving that List, it shouldn’t be up to the controller to handle that. Realistically, SomeDao should deal wih it, but, like I said earlier, the class designer was a jerk.

This architecture is bad in an extreme way, but it still shows that these classes are tightly coupled. The controller is not only tied to the data-access layer, but to that particular implementation of the data access layer… and all because the service layer ducked the exception, trusting that its caller would handle the problem.

There are certainly less insidious and less ridiculous examples that can be thought of, and probably a lot of counterexamples of occasions when it sort of makes sense to duck across a class boundary. However, I think that these can probably be programmed around with careful (re)architecture that ensures that each class and each method is concerned with doing exactly one thing.