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.

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

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

Restricting the number of break and continue statements in a loop is done in the interest of good structured programming.

One break or continue statement is acceptable in a loop, since this allows optimal coding. In most cases, they can either be combined together, or merged with the loop's condition.

The following code:

for (int i = 1; i <= 10; i++) {     // Non-Compliant - 2 continue - one might be tempted to add some logic in between
  if (i % 2 == 0) {
    continue;
  }

  if (i % 3 == 0) {
    continue;
  }

  System.out.println("i = " + i);
}

should be refactored into:

for (int i = 1; i <= 10; i++) {     // Compliant
  if (i % 2 == 0 || i % 3 == 0) {
    continue;
  }

  System.out.println("i = " + i);
}

Avoid using implementation types (i.e., HashSet); use the interface (i.e, Set) instead

This rule is deprecated, use squid:S1319 instead.

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 function/method 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 {
  private int myProperty;

  public int getMyProperty() { // Compliant
    return myProperty;
  }

  public void SetMyProperty(int value) { // Non-Compliant
    myProperty = value;
  }
}

"equals" as a method name should be exclusively used to override Object.equals(Object) to prevent any confusion.

It is tempting to overload the method to take a specific class instead of Object as parameter, to save class comparison check. However, this will not work as expected.

For example:

class MyClass {
  private int foo = 1;

  public boolean equals(MyClass o) {                    // Non-Compliant - "equals" method which does not override Object.equals(Object)
    return o != null && o.foo == this.foo;
  }

  public static void main(String[] args) {
    MyClass o1 = new MyClass();
    Object o2 = new MyClass();
    System.out.println(o1.equals(o2));                  // Will display "false" because "o2" is of type "Object" and not "MyClass"
  }
}

should be refactored into:

class MyClass {
  private int foo = 1;

  @Override
  public boolean equals(Object o) {                     // Compliant - overrides Object.equals(Object)
    if (o == null || !(o instanceof MyClass)) {
      return false;
    }

    MyClass other = (MyClass)o;
    return this.foo == other.foo;
  }

  /* ... */
}

There are three reasons for a method not to have a method body:

• It is an unintentional omission, and should be fixed.
• It is not yet, or never will be, supported. In this case an UnsupportedOperationException should be thrown.
• The method is an intentionally-blank override. In this case a nested comment should explain the reason for the blank override.

The following code snippet:

// Non-Compliant
public void doSomething() {
}

// Compliant
@Override
public void doSomethingElse() {
  // Do nothing because of X and Y.
}

// Compliant
@Override
public void doSomethingElse() {
  throw new UnsupportedOperationException();
}

Empty methods not having any nested comments are tolerated in Abstract classes as those empty methods are usual when implementing the visitor pattern.

Naming a method hashcode() is either:

• A bug. Overriding Object.hashCode() was meant and the application does not behave as expected.
• Done on purpose. The name however will confuse every other developer, who will think it is a bug.

In both cases, the method should be renamed.

The following code:

public int hashcode() { /* ... */ }    // Non-Compliant

should be refactored into:

@Override
public int hashCode() { /* ... */ }    // Compliant

The Cyclomatic Complexity is measured by the number of (&&, ||) operators and (if, while, do, for, ?:, catch, switch, case, return, throw) statements in the body of a class plus one for each constructor, method (but not getter/setter), static initializer, or instance initializer in the class. The last return stament in method, if exists, is not taken into account.

Even when the Cyclomatic Complexity of a class is very high, this complexity might be well distributed among all methods. Nevertheless, most of the time, a very complex class is a class which breaks the Single Responsibility Principle and which should be re-factored to be split in several classes.

A long parameter list can indicate that a new structure should be created to wrap the numerous parameters or that the method is doing too many things.

A class that has private constructors and does not have any static methods or fields cannot be used.

The Java Language Specification recommends listing modifiers in the following order:

• Annotations
• public
• protected
• private
• abstract
• static
• final
• transient
• volatile
• synchronized
• native
• strictfp

Not following this convention has no technical impact, but will reduce the code's readability because most developers are used to the standard order.

The following code:

static public void main(String[] args) {   // Non-Compliant
}

should be refactored into:

public static void main(String[] args) {   // Compliant
}

Non-constructor methods should not have the same name as the enclosing class. Example :

public class MyClass {
  // this is bad because it is a method
  public void MyClass() {}
  // this is OK because it is a constructor
  public MyClass() {}
}

This rule is deprecated, use squid:S1223 instead.

This rule uses the NCSS (Non Commenting Source Statements) algorithm to determine the number of lines of code for a given type. NCSS ignores comments, and counts actual statements. Using this algorithm, lines of code that are split are counted as one.