sebthom/jstuff

Programmers should not comment out code as it bloats programs and reduces readability.

Unused code should be deleted and can be retrieved from source control history if required.

Merging collapsible if statements increases the code's readability.

Noncompliant Code Example

if (file != null) {
  if (file.isFile() || file.isDirectory()) {
    /* ... */
  }
}

Compliant Solution

if (file != null && isFileOrDirectory(file)) {
  /* ... */
}

private static boolean isFileOrDirectory(File file) {
  return file.isFile() || file.isDirectory();
}

Programmers should not comment out code as it bloats programs and reduces readability.

Unused code should be deleted and can be retrieved from source control history if required.

Utility classes, which are collections of static members, are not meant to be instantiated. Even abstract utility classes, which can be extended, should not have public constructors.

Java adds an implicit public constructor to every class which does not define at least one explicitly. Hence, at least one non-public constructor should be defined.

Noncompliant Code Example

class StringUtils { // Noncompliant

  public static String concatenate(String s1, String s2) {
    return s1 + s2;
  }

}

Compliant Solution

class StringUtils { // Compliant

  private StringUtils() {
    throw new IllegalStateException("Utility class");
  }

  public static String concatenate(String s1, String s2) {
    return s1 + s2;
  }

}

Exceptions

When class contains public static void main(String[] args) method it is not considered as utility class and will be ignored by this rule.

Classes in the sun.* or com.sun.* packages are considered implementation details, and are not part of the Java API.

They can cause problems when moving to new versions of Java because there is no backwards compatibility guarantee. Similarly, they can cause problems when moving to a different Java vendor, such as OpenJDK.

Such classes are almost always wrapped by Java API classes that should be used instead.

Noncompliant Code Example

import com.sun.jna.Native;     // Noncompliant
import sun.misc.BASE64Encoder; // Noncompliant

Fields in a Serializable class must themselves be either Serializable or transient even if the class is never explicitly serialized or deserialized. For instance, under load, most J2EE application frameworks flush objects to disk, and an allegedly Serializable object with non-transient, non-serializable data members could cause program crashes, and open the door to attackers. In general a Serializable class is expected to fulfil its contract and not have an unexpected behaviour when an instance is serialized.

This rule raises an issue on non-Serializable fields, and on collection fields when they are not private (because they could be assigned non-Serializable values externally), and when they are assigned non-Serializable types within the class.

Noncompliant Code Example

public class Address {
  //...
}

public class Person implements Serializable {
  private static final long serialVersionUID = 1905122041950251207L;

  private String name;
  private Address address;  // Noncompliant; Address isn't serializable
}

Compliant Solution

public class Address implements Serializable {
  private static final long serialVersionUID = 2405172041950251807L;
}

public class Person implements Serializable {
  private static final long serialVersionUID = 1905122041950251207L;

  private String name;
  private Address address;
}

Exceptions

The alternative to making all members serializable or transient is to implement special methods which take on the responsibility of properly serializing and de-serializing the object. This rule ignores classes which implement the following methods:

 private void writeObject(java.io.ObjectOutputStream out)
     throws IOException
 private void readObject(java.io.ObjectInputStream in)
     throws IOException, ClassNotFoundException;

See

• MITRE, CWE-594 - Saving Unserializable Objects to Disk
• Oracle Java 6, Serializable
• Oracle Java 7, Serializable

When @Overrides of synchronized methods are not themselves synchronized, the result can be improper synchronization as callers rely on the thread-safety promised by the parent class.

Noncompliant Code Example

public class Parent {

  synchronized void foo() {
    //...
  }
}

public class Child extends Parent {

 @Override
  public void foo () {  // Noncompliant
    // ...
    super.foo();
  }
}

Compliant Solution

public class Parent {

  synchronized void foo() {
    //...
  }
}

public class Child extends Parent {

  @Override
  synchronized void foo () {
    // ...
    super.foo();
  }
}

See

• CERT, TSM00-J - Do not override thread-safe methods with methods that are not thread-safe

When @Overrides of synchronized methods are not themselves synchronized, the result can be improper synchronization as callers rely on the thread-safety promised by the parent class.

Noncompliant Code Example

public class Parent {

  synchronized void foo() {
    //...
  }
}

public class Child extends Parent {

 @Override
  public void foo () {  // Noncompliant
    // ...
    super.foo();
  }
}

Compliant Solution

public class Parent {

  synchronized void foo() {
    //...
  }
}

public class Child extends Parent {

  @Override
  synchronized void foo () {
    // ...
    super.foo();
  }
}

See

• CERT, TSM00-J - Do not override thread-safe methods with methods that are not thread-safe

When one part of a getter/setter pair is synchronized the other part should be too. Failure to synchronize both sides of a pair may result in inconsistent behavior at runtime as callers access an inconsistent method state.

This rule raises an issue when either the method or the contents of one method in a getter/setter pair are synchrnoized but the other is not.

Noncompliant Code Example

public class Person {
  String name;
  int age;

  public synchronized void setName(String name) {
    this.name = name;
  }

  public String getName() {  // Noncompliant
    return this.name;
  }

  public void setAge(int age) {  // Noncompliant
    this.age = age;
  }

  public int getAge() {
    synchronized (this) {
      return this.age;
    }
  }
}

Compliant Solution

public class Person {
  String name;
  int age;

  public synchronized void setName(String name) {
    this.name = name;
  }

  public synchronized String getName() {
    return this.name;
  }

  public void setAge(int age) {
    synchronized (this) {
      this.age = age;
   }
  }

  public int getAge() {
    synchronized (this) {
      return this.age;
    }
  }
}

See

• CERT, VNA01-J. - Ensure visibility of shared references to immutable objects

Nesting try/catch blocks severely impacts the readability of source code because it makes it too difficult to understand which block will catch which exception.

Programmers should not comment out code as it bloats programs and reduces readability.

Unused code should be deleted and can be retrieved from source control history if required.

When @Overrides of synchronized methods are not themselves synchronized, the result can be improper synchronization as callers rely on the thread-safety promised by the parent class.

Noncompliant Code Example

public class Parent {

  synchronized void foo() {
    //...
  }
}

public class Child extends Parent {

 @Override
  public void foo () {  // Noncompliant
    // ...
    super.foo();
  }
}

Compliant Solution

public class Parent {

  synchronized void foo() {
    //...
  }
}

public class Child extends Parent {

  @Override
  synchronized void foo () {
    // ...
    super.foo();
  }
}

See

• CERT, TSM00-J - Do not override thread-safe methods with methods that are not thread-safe

Cognitive Complexity is a measure of how hard the control flow of a method is to understand. Methods with high Cognitive Complexity will be difficult to maintain.

See

• Cognitive Complexity

When logging a message there are several important requirements which must be fulfilled:

• The user must be able to easily retrieve the logs
• The format of all logged message must be uniform to allow the user to easily read the log
• Logged data must actually be recorded
• Sensitive data must only be logged securely

If a program directly writes to the standard outputs, there is absolutely no way to comply with those requirements. That's why defining and using a dedicated logger is highly recommended.

Noncompliant Code Example

System.out.println("My Message");  // Noncompliant

Compliant Solution

logger.log("My Message");

See

• CERT, ERR02-J. - Prevent exceptions while logging data

When logging a message there are several important requirements which must be fulfilled:

• The user must be able to easily retrieve the logs
• The format of all logged message must be uniform to allow the user to easily read the log
• Logged data must actually be recorded
• Sensitive data must only be logged securely

If a program directly writes to the standard outputs, there is absolutely no way to comply with those requirements. That's why defining and using a dedicated logger is highly recommended.

Noncompliant Code Example

System.out.println("My Message");  // Noncompliant

Compliant Solution

logger.log("My Message");

See

• CERT, ERR02-J. - Prevent exceptions while logging data

Overriding or shadowing a variable declared in an outer scope can strongly impact the readability, and therefore the maintainability, of a piece of code. Further, it could lead maintainers to introduce bugs because they think they're using one variable but are really using another.

Noncompliant Code Example

class Foo {
  public int myField;

  public void doSomething() {
    int myField = 0;
    ...
  }
}

See

• CERT, DCL01-C. - Do not reuse variable names in subscopes
• CERT, DCL51-J. - Do not shadow or obscure identifiers in subscopes

Non-static initializers are rarely used, and can be confusing for most developers because they only run when new class instances are created. When possible, non-static initializers should be refactored into standard constructors or field initializers.

Noncompliant Code Example

class MyClass {
  private static final Map<String, String> MY_MAP = new HashMap<String, String>() {

    // Noncompliant - HashMap should be extended only to add behavior, not for initialization
    {
      put("a", "b");
    }

  };
}

Compliant Solution

class MyClass {
  private static final Map<String, String> MY_MAP = new HashMap<String, String>();

  static {
    MY_MAP.put("a", "b");
  }
}

or using Java 9 Map.of:

class MyClass {
  // Compliant
  private static final Map<String, String> MY_MAP = java.util.Map.of("a", "b");
}

or using Guava:

class MyClass {
  // Compliant
  private static final Map<String, String> MY_MAP = ImmutableMap.of("a", "b");
}

It is highly recommended not to use wildcard types as return types. Because the type inference rules are fairly complex it is unlikely the user of that API will know how to use it correctly.

Let's take the example of method returning a "List<? extends Animal>". Is it possible on this list to add a Dog, a Cat, ... we simply don't know. And neither does the compiler, which is why it will not allow such a direct use. The use of wildcard types should be limited to method parameters.

This rule raises an issue when a method returns a wildcard type.

Noncompliant Code Example

List<? extends Animal> getAnimals(){...}

Compliant Solution

List<Animal> getAnimals(){...}

or

List<Dog> getAnimals(){...}

The transitive property says that if a == b and b == c, then a == c. In such cases, there's no point in assigning a to c or vice versa because they're already equivalent.

This rule raises an issue when an assignment is useless because the assigned-to variable already holds the value on all execution paths.

Noncompliant Code Example

a = b;
c = a;
b = c; // Noncompliant: c and b are already the same

Compliant Solution

a = b;
c = a;

Fields in a Serializable class must themselves be either Serializable or transient even if the class is never explicitly serialized or deserialized. For instance, under load, most J2EE application frameworks flush objects to disk, and an allegedly Serializable object with non-transient, non-serializable data members could cause program crashes, and open the door to attackers. In general a Serializable class is expected to fulfil its contract and not have an unexpected behaviour when an instance is serialized.

This rule raises an issue on non-Serializable fields, and on collection fields when they are not private (because they could be assigned non-Serializable values externally), and when they are assigned non-Serializable types within the class.

Noncompliant Code Example

public class Address {
  //...
}

public class Person implements Serializable {
  private static final long serialVersionUID = 1905122041950251207L;

  private String name;
  private Address address;  // Noncompliant; Address isn't serializable
}

Compliant Solution

public class Address implements Serializable {
  private static final long serialVersionUID = 2405172041950251807L;
}

public class Person implements Serializable {
  private static final long serialVersionUID = 1905122041950251207L;

  private String name;
  private Address address;
}

Exceptions

The alternative to making all members serializable or transient is to implement special methods which take on the responsibility of properly serializing and de-serializing the object. This rule ignores classes which implement the following methods:

 private void writeObject(java.io.ObjectOutputStream out)
     throws IOException
 private void readObject(java.io.ObjectInputStream in)
     throws IOException, ClassNotFoundException;

See

• MITRE, CWE-594 - Saving Unserializable Objects to Disk
• Oracle Java 6, Serializable
• Oracle Java 7, Serializable