Why shouldn't a method throw multiple types of checked exceptions?

Question

We use SonarQube to analyse our Java code and it has this rule (set to critical):

Public methods should throw at most one checked exception

Using checked exceptions forces method callers to deal with errors, either by propagating them or by handling them. This makes those exceptions fully part of the API of the method.

To keep the complexity for callers reasonable, methods should not throw more than one kind of checked exception."

Another bit in Sonar has this:

Public methods should throw at most one checked exception

Using checked exceptions forces method callers to deal with errors, either by propagating them or by handling them. This makes those exceptions fully part of the API of the method.

To keep the complexity for callers reasonable, methods should not throw more than one kind of checked exception.

The following code:

public void delete() throws IOException, SQLException {      // Non-Compliant
  /* ... */
}

should be refactored into:

public void delete() throws SomeApplicationLevelException {  // Compliant
    /* ... */
}

Overriding methods are not checked by this rule and are allowed to throw several checked exceptions.

I've never come across this rule/recommendation in my readings on exception handling and have tried to find some standards, discussions etc. on the topic. The only thing I've found is this from CodeRach: How many exceptions should a method throw at most?

Is this a well accepted standard?

Answer 1

Lets consider the situation where you have the provided code of:

public void delete() throws IOException, SQLException {      // Non-Compliant
  /* ... */
}

The danger here is that the code that you write to call delete() will look like:

try {
  foo.delete()
} catch (Exception e) {
  /* ... */
}

This is bad too. And it will be caught with another rule that flags catching the base Exception class.

The key is to not write code that makes you want to write bad code elsewhere.

The rule that you are encountering is a rather common one. Checkstyle has it in its design rules:

ThrowsCount

Restricts throws statements to a specified count (1 by default).

Rationale: Exceptions form part of a method's interface. Declaring a method to throw too many differently rooted exceptions makes exception handling onerous and leads to poor programming practices such as writing code like catch(Exception ex). This check forces developers to put exceptions into a hierarchy such that in the simplest case, only one type of exception need be checked for by a caller but any subclasses can be caught specifically if necessary.

This precisely describes the problem and what the issue is and why you shouldn't do it. It is a well accepted standard that many static analysis tools will identify and flag.

And while you may do it according to language design, and there may be times when it is the right thing to do, it is something that you should see and immediately go "um, why am I doing this?" It may be acceptable for internal code where everyone is disciplined enough to never catch (Exception e) {}, but more often than not I've seen people cut corners especially in internal situations.

Don't make people using your class want to write bad code.

---

I should point out that the importance of this is lessened with Java SE 7 and later because a single catch statement can catch multiple exceptions (Catching Multiple Exception Types and Rethrowing Exceptions with Improved Type Checking from Oracle).

With Java 6 and before, you would have code that looked like:

public void delete() throws IOException, SQLException {
  /* ... */
}

and

try {
  foo.delete()
} catch (IOException ex) {
     logger.log(ex);
     throw ex;
} catch (SQLException ex) {
     logger.log(ex);
     throw ex;
}

or

try {
    foo.delete()
} catch (Exception ex) {
    logger.log(ex);
    throw ex;
}

Neither of these options with Java 6 are ideal. The first approach violates DRY. Multiple blocks doing the same thing, again and again - once for each exception. You want to log the exception and rethrow it? Ok. The same lines of code for each exception.

The second option is worse for several reasons. First, it means that you are catching all the exceptions. Null pointer gets caught there (and it shouldn't). Furthermore, you are rethrowing an Exception which means that the method signature would be deleteSomething() throws Exception which just makes a mess further up the stack as people using your code are now forced to catch(Exception e).

With Java 7, this isn't as important because you can instead do:

catch (IOException|SQLException ex) {
    logger.log(ex);
    throw ex;
}

Furthermore, the type checking if one does catch the types of the exceptions being thrown:

public void rethrowException(String exceptionName)
throws IOException, SQLException {
    try {
        foo.delete();
    } catch (Exception e) {
        throw e;
    }
}

The type checker will recognize that e may only be of types IOException or SQLException. I'm still not overly enthusiastic about the use of this style, but it isn't causing as bad code as it was under Java 6 (where it would force you to have the method signature be the superclass that the exceptions extend).

Despite all these changes, many static analysis tools (Sonar, PMD, Checkstyle) are still enforcing Java 6 style guides. It's not a bad thing. I tend to agree with a warning these to still be enforced, but you might change the priority on them to major or minor according to how your team prioritizes them.

If exceptions should be checked or unchecked... that is a matter of great debate that one can easily find countless blog posts taking up each side of the argument. However, if you are working with checked exceptions, you probably should avoid throwing multiple types, at least under Java 6.

Answer 2

The reason that you would, ideally, want to only throw one type of exception is because doing otherwise likely violates the Single Responsibility and Dependency Inversion principles. Let's use an example to demonstrate.

Let's say we have a method that fetches data from persistence, and that persistence is a set of files. Since we are dealing with files, we can have a FileNotFoundException:

public String getData(int id) throws FileNotFoundException

Now, we have a change in requirements, and our data comes from a database. Instead of a FileNotFoundException (since we are not dealing with files), we now throw an SQLException:

public String getData(int id) throws SQLException

We would now have to go through all code that uses our method and change the exception we have to check for, else the code won't compile. If our method gets called far and wide, that can be a lot to change/have others change. It takes a lot of time, and people aren't going to be happy.

Dependency inversion says that we really shouldn't throw either of these exceptions because they expose internal implementation details we are working to encapsulate. Calling code needs to know what type of persistence we are using, when it really should just be worried about if the record can be retrieved. Instead we should throw an exception that conveys the error at the same level of abstraction as we are exposing through our API:

public String getData(int id) throws InvalidRecordException

Now, if we change the internal implementation, we can simply wrap that exception in an InvalidRecordException and pass it along (or not wrap it, and just throw a new InvalidRecordException). External code does not know or care what type of persistence is being used. It's all encapsulated.

---

As for Single Responsibility, we need to think about code that throws multiple, unrelated exceptions. Let's say we have the following method:

public Record parseFile(String filename) throws IOException, ParseException

What can we say about this method? We can tell just from the signature that it opens a file and parses it. When we see a conjunction, like "and" or "or" in the description of a method, we know that it is doing more than one thing; it has more than one responsibility. Methods with more than one responsibility are hard to manage as they can change if any of the responsibilities change. Instead, we should break methods up so they have a single responsibility:

public String readFile(String filename) throws IOException
public Record parse(String data) throws ParseException

We've extracted out the responsibility of reading the file from the responsibility of parsing the data. One side effect of this is that we can now pass in any String data to the parse data from any source: in-memory, file, network, etc. We can also test parse more easily now because we don't need a file on disk to run tests against.

---

Sometimes there really are two (or more) exceptions we can throw from a method, but if we stick to SRP and DIP, the times we encounter this situation become rarer.

Answer 3

I remember fiddling around with this a little bit when playing with Java a while back, but I wasn't real conscious of the distinction between checked and unchecked until I read your question. I found this article on Google pretty quickly, and it goes into some of the apparent controversy:

http://tutorials.jenkov.com/java-exception-handling/checked-or-unchecked-exceptions.html

That being said, one of the issues this guy was mentioning with checked exceptions is that (and I've personally run into this from the beginning with Java) if you keep adding a bunch of checked exceptions to throws clauses in your method declarations, not only do you have to put in a lot more boilerplate code to support it as you move to higher-level methods, but it also just makes a bigger mess and breaks compatibility when you try to introduce more exception types to lower-level methods. If you add a checked exception type to a lower-level method, then you have to run back through your code and adjust several other method declarations as well.

One point of mitigation mentioned in the article - and the author did not like this personally - was to create a base class exception, limit your throws clauses to only use it, and then just raise subclasses of it internally. That way you can create new checked exception types without having to run back through all your code.

The author of this article may not have liked this much, but it makes perfect sense in my personal experience (especially if you can look up what all the subclasses are anyway), and I bet that's why the advice you're being given is to cap everything to one checked exception type each. What's more is that the advice you mentioned actually allows for multiple checked exception types in non-public methods, which makes perfect sense if this is their motive (or even otherwise). If it's just a private method or something similar anyway, you're not going to have run through half your codebase when you change one little thing.

You did largely ask if this is an accepted standard, but between your research you mentioned, this reasonably thought-out article, and just speaking from personal programming experience, it certainly doesn't seem to stand out in any way.

Answer 4

Throwing multiple checked exceptions make sense when there is multiple reasonable things to do.

For example lets say you have method

public void doSomething(Credentials cred, Work work) 
    throws CredentialsRequiredException, TryAgainLaterException{...}

this violates pne exception rule, but makes sense.

Unfortunatelly, what usuallly happens are methods like

void doSomething() 
    throws IOException, JAXBException,SQLException,MyException {...}

Here there is little chance for caller to do something specific based on exception type. So if we want to nudge him into realizing that these methods CAN and sometimes WILL go wrong, throwing just SomethingMightGoWrongException is enogh and better.

Hence rule at most one checked exception.

But if your project use design where there are multiple meaningfull checked exceptions, this rule should not apply.

Sidenote: Something actually can go wrong almost everywhere, so one can think about using ? extends RuntimeException, but there is difference between "we all do mistakes" and "this talks external system and it sometimes WILL be down, deal with it".