Error Handling

Best practices

Return empty arrays or collections, not nulls.

(c) Joshua Bloch, Effective Java

TODO: investigate what can be done in "Sonar for Sonar" profile to enforce this rule.

Every parameter is non-null unless explicitly specified.

Package which follows this rule should contain package-info.java :

@ParametersAreNonnullByDefault
package packageName;
 
import javax.annotation.ParametersAreNonnullByDefault;

Form of nullability declaration :

import javax.annotation.Nullable;
Type methodName(@Nullable Type parameter) {
  ...
}

Note that main point in this statement is not an annotation, but a principle.

Throw early and use Preconditions.

Preconditions on parameters are required on API to establish contract between API and consumer of API. Then it's not mandatory for backend to verify again that parameters are correct.

void parse(String filename) {
  Preconditions.checkNotNull(filename, "filename can't be null"); // otherwise stack-strace will contain line inside constructor of FileInputStream, when we will execute it
  ... // maybe huge amount of code
  InputStream is = new FileInputStream(filename);
  ...
}

Also this means that NullPointerException, IllegalArgumentException and IllegalStateException are technical exceptions and should not be shown in their original form to the end-user : compare "NullPointerException: filename can't be null" and "Unable to execute sensor Surefire: NullPointerException". Thus even short form of precondition without message is efficient :

Preconditions.checkNotNull(parameter);
Preconditions.checkArgument(expression);
Preconditions.checkState(expression);

Every class is not thread safe unless explicitly specified.

Form of declaration :

import javax.annotation.concurrent.ThreadSafe;
 
@ThreadSafe
class ClassName {
  ...
}

Note that immutable classes are thread safe by definition and so this declaration is redundant for them.

Implementations of custom Exceptions must be immutable.

Use ExpectedException rule from JUnit for assertions on Exception.

This JUnit rule replaces the property "expected" of the annotation @Rule.

@Rule
public ExpectedException thrown = ExpectedException.none();
 
@Test
public void test() {
  thrown.expect(SonarException.class);
  thrown.expectMessage("message");
  someMethod(); // throws SonarException
}

Annotate methods which visibility is relaxed to make the code testable

Use the Guava annotation @VisibleForTesting :

@com.google.common.annotations.VisibleForTesting
String packageVisibleBecauseOfUnitTest() {
}

Anti patterns

Log and Throw / Rethrow.

Exceptions as flow control.

Declaring exceptions other than Exception in signatures of test methods.

Catch and Ignore.

(c) Joshua Bloch, Effective Java, Item 65

When a method from some API throws a checked exception, it's trying to tell you that you should take some counter action. Do not ignore this by catching in empty block and continue as if nothing happened !

First of all - if you can recover, then recover !

Second - if checked exception does not make sense to you, then do not hesitate to convert it into an unchecked exception and throw it again : 

try {
  someMethod();
} catch (CheckedException e) {
  throw SomeRuntimeException(...);
}

As a last resort - use logging :

try {
  someMethod();
} catch (CheckedException e) {
  LOG.warn(...);
}

Destructive wrapping.

Destructive wrapping means that original exception will be lost together with all information like stack trace. Forms of destructive wrapping :

try {
  ...
} catch (SomeException e) {
  throw new AnotherException();
  throw new AnotherException(message);
  throw new AnotherException(e.getMessage());
}

Original exception should be always added into wrapped exception. If such constructor does not exist, then use method initCause, which is always available, because declared in Throwable, e.g. :

try {
  ...
} catch (SomeException e) {
  javax.persistence.NonUniqueResultException e = new NonUniqueResultException();
  e.initCause(e);
  throw e;
}

Throw from Within Finally.

Following code is fine as long as cleanUp() can never throw an exception :

try {
  someMethod();
} finally {
  cleanUp();
}

If someMethod() throws an exception, and then cleanUp() throws an exception, then second exception will swallow first one. If cleanUp() can possibly throw an exception, then make sure that you either handle it, or log it.

Return from Finally.

This can discard exceptions.

Returning null when operation is unsupported.

Instead - throw an exception.