(c) Joshua Bloch, Effective Java
TODO: investigate what can be done in "Sonar for Sonar" profile to enforce this rule.
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.
|
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); |
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.
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
} |
Use the Guava annotation @VisibleForTesting :
@com.google.common.annotations.VisibleForTesting
String packageVisibleBecauseOfUnitTest() {
} |
ExceptionAsFlowControl |
(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 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;
} |
AvoidLosingExceptionInformation |
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.
DoNotThrowExceptionInFinally |
This can discard exceptions.
ReturnFromFinallyBlock |
Instead - throw an exception.