Skip to end of metadata
Go to start of metadata

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 :

Form of nullability declaration :

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

 Findbugs Rules
  • NP_NULL_PARAM_DEREF_ALL_TARGETS_DANGEROUS ( Correctness - Method call passes null for nonnull parameter (ALL_TARGETS_DANGEROUS) )
  • NP_NONNULL_PARAM_VIOLATION ( Correctness - Method call passes null to a nonnull parameter )
  • NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE ( Dodgy - Parameter must be nonnull but is marked as nullable )
  • NP_STORE_INTO_NONNULL_FIELD ( Correctness - Store of null value into field annotated NonNull )
  • NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE ( Dodgy - Possible null pointer dereference due to return value of called method )
  • NP_NONNULL_RETURN_VIOLATION ( Correctness - Method may return null, but is declared @NonNull )
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.

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 :

Every class is not thread safe unless explicitly specified.

Form of declaration :

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.

Annotate methods which visibility is relaxed to make the code testable

Use the Guava annotation @VisibleForTesting :

Anti patterns

Log and Throw / Rethrow.
Exceptions as flow control.
 PMD Rule

 ExceptionAsFlowControl

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 : 

Good

As a last resort - use logging :

Good enough
 Findbugs Rules
  • DE_MIGHT_IGNORE
  • DE_MIGHT_DROP
Destructive wrapping.

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

Bad

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. :

Good
 PMD Rule

AvoidLosingExceptionInformation

Throw from Within Finally.

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

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.

 PMD Rule

DoNotThrowExceptionInFinally

Return from Finally.

This can discard exceptions.

 PMD Rule

ReturnFromFinallyBlock

Returning null when operation is unsupported.

Instead - throw an exception.

Labels
  • None