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.

OpenJDK introduces a potential incompatibility. In particular, the java.util.logging.Logger behavior has changed. Instead of using strong references, it now uses weak references internally. That's a reasonable change, but unfortunately some code relies on the old behavior - when changing logger configuration, it simply drops the logger reference. That means that the garbage collector is free to reclaim that memory, which means that the logger configuration is lost. For example, consider:

public static void initLogging() throws Exception {
      Logger logger = Logger.getLogger("edu.umd.cs");
      logger.addHandler(new FileHandler()); // call to change logger configuration
      logger.setUseParentHandlers(false); // another call to change logger configuration
      }

The logger reference is lost at the end of the method (it doesn't escape the method), so if you have a garbage collection cycle just after the call to initLogging, the logger configuration is lost (because Logger only keeps weak references).

public static void main(String[] args) throws Exception {
      initLogging(); // adds a file handler to the logger
      System.gc(); // logger configuration lost
      Logger.getLogger("edu.umd.cs").info("Some message"); // this isn't logged to the file as expected
      }

Ulf Ochsenfahrt and Eric Fellheimer

The complexity of an expression is defined by the number of &&, || and condition ? ifTrue : ifFalse operators it contains. A single expression's complexity should not become too high to keep the code readable.

The following code, with a maximum complexity of 3:

if (condition1 && condition2 && condition3 && condition4) { /* ... */ }  // Non-Compliant

could be refactored into something like:

if (relevantMethodName1() && relevantMethodName2()) { /* ... */ }        // Compliant

/* ... */

private boolean relevantMethodName1() {
  return condition1 && condition2;
}

private boolean relevantMethodName2() {
  return condition3 && condition4;
}

This static field public but not final, and could be changed by malicious code or by accident from another package. The field could be made final to avoid this vulnerability. However, the static initializer contains more than one write to the field, so doing so will require some refactoring.

Sharing some naming conventions is a key point to make it possible for a team to efficiently collaborate. This rule allows to check that all field names match a provided regular expression.

The following code snippet illustrates this rule when the regular expression value is "^[a-z][a-zA-Z0-9]*$":

public class MyClass {
  public int FIRST = 1; // Non-Compliant
  public int second = 2; // Compliant
}

Names of fields that are not final should be in mixed case with a lowercase first letter and the first letters of subsequent words capitalized.

FIXME tags are commonly used to mark places where a bug is suspected, but which the developer wants to deal with later. Sometimes the developer will not have the time or will simply forget to get back to that tag. This rule is meant to track those tags, and ensure that they do not go unnoticed.

The following code illustrates this rule:

int divide(int numerator, int denominator) {
  return numerator / denominator;              // FIXME denominator = 0, Non-Compliant
}

This format string include a newline character (\n). In format strings, it is generally preferable better to use %n, which will produce the platform-specific line separator.

Using such generic exception prevents calling methods from handling differently each kind of error.

The following code snippet illustrates this rule:

public void foo(String bar) throws Throwable { // Non-Compliant
  throw new RuntimeException("My Message");    // Non-Compliant
}

public void foo(String bar) {
  throw new MyRuntimeException("My Message");  // Compliant
}

Avoid idempotent operations - they are have no effect. Example :
int x = 2;
x = x;

If statements with conditions that are always either true or false are not required, and make the code less readable.

The following code:

public void myMethod() {
  if (true) {                   // Non-Compliant
    doSomething();
  }
}

should be refactored into:

public void myMethod() {
  doSomething();                // Compliant
}

and the following code:

public void myMethod() {
  if (false) {                  // Non-Compliant
    doSomething();
  }
}

should be refactored into:

public void myMethod() {        // Compliant
}

Not using curly braces could be error-prone in some cases. For instance in the following example, the two statements seems to be attached to the if statement whereas this is the case only for the first one:

if (condition) // Non-Compliant
  executeSomething();
  checkSomething();

if (condition) { // Compliant
  executeSomething();
}
checkSomething();

Avoid concatenating non literals in a StringBuffer constructor or append().

Avoid instantiating an object just to call getClass() on it; use the .class public member instead. Example : replace Class c = new String().getClass(); with Class c = String.class;

An issue is created on a file as soon as the branch coverage on this file is less than the required threshold. It gives the number of branches to be covered in order to reach the required threshold.

This code converts a 32-bit int value to a 64-bit long value, and then passes that value for a method parameter that requires an absolute time value. An absolute time value is the number of milliseconds since the standard base time known as "the epoch", namely January 1, 1970, 00:00:00 GMT. For example, the following method, intended to convert seconds since the epoc into a Date, is badly broken:

Date getDate(int seconds) { return new Date(seconds * 1000); }

The multiplication is done using 32-bit arithmetic, and then converted to a 64-bit value. When a 32-bit value is converted to 64-bits and used to express an absolute time value, only dates in December 1969 and January 1970 can be represented.

Correct implementations for the above method are:

// Fails for dates after 2037
Date getDate(int seconds) { return new Date(seconds * 1000L); }

// better, works for all dates
Date getDate(long seconds) { return new Date(seconds * 1000); }

In JDK 1.5, calling new Integer() causes memory allocation. Integer.valueOf() is more memory friendly.

Sharing some naming conventions is a key point to make it possible for a team to efficiently collaborate. This rule allows to check that all interface names match a provided regular expression.

The following code snippet illustrates this rule when the regular expression value is "^[A-Z][a-zA-Z0-9]*$":

public interface MyFirstInterface {...} // Compliant
public interface mySecondInterface {...} // Non-Compliant

A String is being converted to upper or lowercase, using the platform's default encoding. This may result in improper conversions when used with international characters. Use the

versions instead.

Labels are not commonly used in Java, and many developers do not understand how they work. Moreover, their usage make the control flow harder to follow, which reduces the code's readability.

The following code:

int matrix[][] = {
  {1, 2, 3},
  {4, 5, 6},
  {7, 8, 9}
};

outer: for (int row = 0; row < matrix.length; row++) {   // Non-Compliant
  for (int col = 0; col < matrix[row].length; col++) {
    if (col == row) {
      continue outer;
    }
    System.out.println(matrix[row][col]);                // Prints the elements under the diagonal, i.e. 4, 7 and 8
  }
}

should be refactored into:

for (int row = 1; row < matrix.length; row++) {          // Compliant
  for (int col = 0; col < row; col++) {
    System.out.println(matrix[row][col]);                // Also prints 4, 7 and 8
  }
}

Sharing some coding conventions is a key point to make it possible for a team to efficiently collaborate. This rule make it mandatory to place left curly braces at the end of lines of code.

The following code snippet illustrates this rule:

public void myMethod() {              // Compliant
  if(something)
  {                                   // Non-Compliant
    executeTask();
  } else {                            // Compliant
    doSomethingElse();
  }
  if( param1 && param2 && param3
    && something3 && something4)
  {                                   // Non-Compliant
    executeAnotherTask();
  }
}

Sharing some naming conventions is a key point to make it possible for a team to efficiently collaborate. This rule allows to check that all local variable and method parameter names match a provided regular expression.

The following code snippet illustrates this rule when the regular expression value is "^[a-z][a-zA-Z0-9]*$":

public class MyClass {
  public void method() {
    int FIRST;                      // Non-Compliant
    int second;                     // Compliant
  }

  public void method2(int VALUE) {  // Non-Compliant
  }

  public void method3(int value) {  // Compliant
  }
}

Shadowing fields with a local variable is a bad practice reducing code readability: It makes it confusing to know whether the field or the variable is and should be accessed.

The following code illustrates this rule:

class Foo {
  public int myField;

  public Foo(int myField) {                   // Compliant - method parameters are not checked
    this.myField = myField;
  }

  @Override
  public String toString() {
    int myField = 0;                          // Non-Compliant - should be renamed
    return "Foo{MyField: " + myField + "}";
  }
}

The long suffix should always be written in upper case, i.e. 'L', as the lower case 'l' can easily be confused with the digit one '1'.

The following code:

long n = 10l;  // Non-Compliant - easily confused with one zero one

should be refactored into:

long n = 10L;  // Compliant

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);
}