Java – Why Methods Shouldn’t Throw Multiple Types of Checked Exceptions

exceptionsjavastatic analysis

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?

Best Answer

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.

Related Topic