| |
"equals(Object obj)" and "hashCode()" should be overridden in pairs
According to the Java Language Specification, there is a contract between equals(Object) and hashCode():
If two objects are equal according to the equals(Object) method, then calling the hashCode method on each of the two objects must produce the same integer result. It is not required that if two objects are unequal according to the equals(java.lang.Object) method, then calling the hashCode method on each of the two objects must produce distinct integer results. However, the programmer should be aware that producing distinct integer results for unequal objects may improve the performance of hashtables.
In order to comply with this contract, those methods should be either both inherited, or both overridden.
Noncompliant Code Example
class MyClass { // Non-Compliant - should also override "hashCode()"
@Override
public boolean equals(Object obj) {
/* ... */
}
}
Compliant Solution
class MyClass { // Compliant
@Override
public boolean equals(Object obj) {
/* ... */
}
@Override
public int hashCode() {
/* ... */
}
}
|
|
"equals(Object obj)" should be overridden along with the "compareTo(T obj)" method
According to the Java Comparable.compareTo(T o) documentation:
It is strongly recommended, but not strictly required that (x.compareTo(y)==0) == (x.equals(y)). Generally speaking, any class that implements the Comparable interface and violates this condition should clearly indicate this fact. The recommended language is "Note: this class has a natural ordering that is inconsistent with equals."
If this rule is violated, weird and unpredictable failures can occur. For example, in Java 5 the PriorityQueue.remove() method relied on compareTo(), but since Java 6 it relies on equals().
Noncompliant Code Example
public class Foo implements Comparable<Foo> {
@Override
public int compareTo(Foo foo) { /* ... */ } // Noncompliant as the equals(Object obj) method is not overridden
}
Compliant Solution
public class Foo implements Comparable<Foo> {
@Override
public int compareTo(Foo foo) { /* ... */ } // Compliant
@Override
public boolean equals(Object obj) { /* ... */ }
}
|
|
Execution of the Garbage Collector should be triggered only by the JVM
Calling System.gc() or Runtime.getRuntime().gc() is a bad idea for a simple reason: there is no way to know exactly what will be done under the hood by the JVM because the behavior will depend on its vendor, version and options:
- Will the whole application be frozen during the call?
- Is the -XX:DisableExplicitGC option activated?
- Will the JVM simply ignore the call?
- ...
An application relying on those unpredictable methods is also unpredictable and therefore broken.
The task of running the garbage collector should be left exclusively to the JVM.
|
|
Loop counters should not be assigned to from within the loop body
Loop counters should not be modified in the body of the loop. However other loop control variables representing logical values may be modified in the loop, for example a flag to indicate that something has been completed, which is then tested in the for statement.
The following code:
String[] names = new String[]{ "Jack", "Jim", null, "John" };
for (int i = 0; i < names.length; i++) {
if (names[i] == null) {
i = names.length; // Non-Compliant
} else {
System.out.println(names[i]);
}
}
should be refactored into:
String[] names = new String[]{ "Jack", "Jim", null, "John" };
for (String name: names) {
if (name == null) {
break; // Compliant
}
System.out.println(name);
}
|
|
Nested blocks of code should not be left empty
Most of the time a block of code is empty when a piece of code is really missing. So such empty block must be either filled or removed. When a block contains a comment, this block is not considered to be empty.
The following code snippet illustrates this rule:
void doSomething() {
for (int i = 0; i < 42; i++) // Non-Compliant
{
}
for (int i = 0; i < 42; i++); // Compliant
if (myVar == 4) // Compliant - contains a comment
{
// Do nothing because of X and Y
}
else // Compliant
{
doSomething();
}
try // Non-Compliant
{
}
catch (Exception e) // Compliant
{
// Ignore
}
}
|
|
Return statements should not occur in finally blocks
Returning from a finally block suppresses the propagation of any unhandled Throwable which was thrown in the try or catch block.
The following code snippet illustrates this rule. The developer expects to get "ERROR" in the console whereas "OK" is displayed.
public static void main(String[] args) {
try {
doSomethingWhichThrowsException();
System.out.println("OK");
} catch (RuntimeException e) {
System.out.println("ERROR");
}
}
public static void doSomethingWhichThrowsException() {
try {
throw new RuntimeException();
} finally {
/* ... */
return; // Non-Compliant - prevents the RuntimeException from being propagated
}
}
|
|
Strings should be compared using equals()
String literals, just like any other Object, should be compared using the equals() method. Using == and != does not work in general.
The following code:
if (variable == "foo") { /* ... */ } // Non-Compliant
if (variable != "foo") { /* ... */ } // Non-Compliant
should be refactored into:
if ("foo".equals(variable)) { /* ... */ } // Compliant
if (!"foo".equals(variable)) { /* ... */ } // Compliant
|
|
super.finalize() should be called at the end of Object.finalize() implementations
Overriding the Object.finalize() method must be done with caution to dispose some system resources. Calling the super.finalize() at the end of this method implementation is highly recommended in case parent implementations must also dispose some system resources.
The following code snippet illustrates this rule:
protected void finalize() { // Non-Compliant
releaseSomeResources();
}
protected void finalize() {
super.finalize(); // Non-Compliant
releaseSomeResources();
}
protected void finalize() {
releaseSomeResources();
super.finalize(); // Compliant
}
|
