Java Coding Violations
Avoid Branching Statement As Last In Loop
Description: Using a branching statement as the last part of a loop may be a bug, and/or is confusing
for (int j = 0; j < 5; j++) {
if (j*j <= 12) {
continue;
}
break;
}
for (int j = 0; j < 5; j++) {
if (j*j > 12) {
break;
}
}
Avoid Decimal Literals In Big Decimal Constructor
Description: "One might assume that the result of ""new BigDecimal(0.1)"" is exactly equal to 0.1, but it is not because 0.1 cannot be represented exactly as a double (or as a binary fraction of any finite length)"
BigDecimal bd = new BigDecimal(2.456);
BigDecimal bd = new BigDecimal("2.456");
Avoid Multiple Unary Operators
Description: Avoid Multiple Unary Operators
int k = - -1;
int l = + - +1;
int m = ~~4;
boolean a = !!true;
boolean b = !!!true;
Avoid Thread Group
Description: Avoid using java.lang.ThreadGroup. Although it is intended to be used in a threaded environment it contains methods that are not thread-safe
public class Beta {
void beta() {
ThreadGroup tg = new ThreadGroup("My threadgroup");
tg = new ThreadGroup(tg, "my thread group");
tg = Thread.currentThread().getThreadGroup();
tg = System.getSecurityManager().getThreadGroup();
}
}
Avoid Using Hard Coded IP
Description: Application with hard-coded IP addresses can become impossible to deploy in some cases. Externalizing IP adresses is preferable
public class Alpha {
private String ip = "127.0.0.1";
}
Avoid Using Octal Values
Description: Integer literals should not start with zero since this denotes that the rest of literal will be interpreted as an octal value
int i = 010;
int i = 10;
Big Integer Instantiation
Description: Don’t create instances of already existing BigInteger
BigInteger bf = new BigInteger(1);
bf = BigInteger.ONE;
Boolean Instantiation
Description: Avoid instantiating Boolean objects
Boolean foo = new Boolean("true");
Broken Null Check
Description: The null check is broken since it will throw a NullPointerException itself
public String foo(String string) {
if (string!=null || !string.equals(""))
return string;
if (string==null && string.equals(""))
return string;
}
public String foo(String string) {
if (string!=null && !string.equals(""))
return string;
if (string==null || string.equals(""))
return string;
}
Check Result Set
Description: Always check the return values of navigation methods (next, previous, first, last) of a ResultSet. If the value return is ‘false’, it should be handled properly
Statement stat = conn.createStatement();
ResultSet rst = stat.executeQuery("SELECT testvalue FROM testtable");
rst.next();
String testName = rst.getString(1);
Statement stat = conn.createStatement();
ResultSet rst = stat.executeQuery("SELECT name FROM person");
if (rst.next()) {
String firstName = rst.getString(1);
} else {
// handle else
}
Check Skip Result
Description: The skip() method may skip a smaller number of bytes than requested
public class Alpha {
private FileInputStream _s = new FileInputStream("file");
public void skip(int n) throws IOException {
_s.skip(n);
}
}
public class Alpha {
private FileInputStream _s = new FileInputStream("file");
public void skip(int n) throws IOException {
_s.skip(n);
}
public void skipExactly(int n) throws IOException {
while (n != 1) {
long skipped = _s.skip(n);
if (skipped == 1)
throw new EOFException();
n -= skipped;
}
}
}
Class Cast Exception With To Array
Description: When deriving an array of a specific class from your Collection, one should provide an array of the same class as the parameter of the toArray() method. Doing otherwise you will will result in a ClassCastException
Collection a = new ArrayList();
Integer obj = new Integer(1);
a.add(obj);
Integer[] b = (Integer [])a.toArray();
Collection a = new ArrayList();
Integer obj = new Integer(1);
a.add(obj);
Integer[] b = (Integer [])a.toArray(new Integer[a.size()]);
Collapsible If Statements
Description: Sometimes two consecutive ‘if’ statements can be consolidated by separating their conditions with a boolean short-circuit operator
void foo() {
if (a) {
if (b) {
// doSomething
}
}
}
void foo() {
if (a && b) {
// doSomething
}
}
Explicitly calling Thread.run()
Description: Explicitly calling Thread.run() method will execute in the caller’s thread of control. Instead, call Thread.start() for the intended behavior
Thread a = new Thread();
a.run();
Thread a = new Thread();
a.start();
Dont Use Float Type For Loop Indices
Description: Don’t use floating point for loop indices
public class Count {
public static void main(String[] args) {
final int START = 5000000000;
int count = 0;
for (float f = START; f < START + 50; f++)
count++;
System.out.println(count);
}
}
Double Checked Locking
Description: Partially created objects can be returned by the Double Checked Locking pattern when used in Java
public class Alpha {
Object beta = null;
Object gamma() {
if (beta == null) {
synchronized(this) {
if (beta == null) {
beta = new Object();
}
}
}
return beta;
}
}
public class Alpha {
/*volatile */ Object beta = null;
Object gamma() {
if (beta == null) {
synchronized(this) {
if (beta == null) {
beta = new Object();
}
}
}
return beta;
}
}
Empty Catch Block
Description: Empty Catch Block finds instances where an exception is caught, but nothing is done
public void doThis() {
try {
FileInputStream fil = new FileInputStream("/tmp/test");
} catch (IOException ioe) {
}
}
Empty Finally Block
Description: Empty finally blocks serve no purpose and should be removed
public class Alpha {
public void beta() {
try {
int x = 5;
} finally {
// empty!
}
}
}
public class Alpha {
public void beta() {
try {
int x = 5;
}
}
}
Empty If Stmt
Description: Empty If Statement finds instances where a condition is checked but nothing is done about it
public class Alpha {
void beta(int y) {
if (y == 2) {
// empty!
}
}
}
public class Alpha {
void beta(int y) {
if (y == 2) {
//doSomething
}
}
}
Empty Statement Block
Description: Empty block statements serve no purpose and should be removed.
public class Alpha {
private int _beta;
public void setBeta(int beta) {
{ _beta = beta; }
{}
}
}
public class Alpha {
private int _beta;
public void setBeta(int beta) {
{ _beta = beta; }
}
}
Empty Statement Not In Loop
Description: An empty statement (or a semicolon by itself) that is not used as the sole body of a ‘for’ or ‘while’ loop is probably a bug
public void doThis() {
System.out.println("look at the extra semicolon");;
}
public void doThis() {
System.out.println("look at the extra semicolon");
}
Empty Static Initializer
Description: Empty initializers serve no purpose and should be removed
public class Alpha {
static {}
}
public class Alpha {
static {
//doSomething
}
}
Empty Switch Statements
Description: Empty switch statements serve no purpose and should be removed
public void beta() {
int a = 5;
switch (a) {}
}
public void beta() {
int a = 5;
switch (a) {
//doSomething
}
}
Empty Synchronized Block
Description: Empty synchronized blocks serve no purpose and should be removed
public class Alpha {
public void beta() {
synchronized (this) {
}
}
}
public class Alpha {
public void beta() {
synchronized (this) {
//doSomething
}
}
}
Empty Try Block
Description: Avoid empty try blocks
public class Alpha {
public void beta() {
try {
} catch (Exception e) {
e.printStackTrace();
}
}
}
Empty While Stmt
Description: Empty While Statement finds all instances where a while statement does nothing.If it is a timing loop, then you should use Thread.sleep() for it
void beta(int x, int y) {
while (x == y) {
}
}
void beta(int x, int y) {
while (x == y) {
//doSomething
}
}
Extends Object
Description: No need to explicitly extend Object
public class Alpha extends Object {
}
For Loop Should Be While Loop
Description: Some for loops can be simplified to while loops, this makes them more concise
public class Alpha {
void beta() {
for (;true;) true;
}
}
public class Alpha {
void beta() {
while (true) true;
}
}
Jumbled Incrementer
Description: Avoid jumbled loop incrementers - its usually a mistake, and is confusing even if intentional
public class Alpha {
public void beta() {
for (int j = 0; j < 5; j++) {
for (int k = 0; k < 12; j++) {
System.out.println("Hello World");
}
}
}
}
public class Alpha {
public void beta() {
for (int j = 0; j < 5; j++) {
for (int k = 0; k < 12; k++) {
System.out.println("Hello World");
}
}
}
}
Misplaced Null Check
Description: "The null check here is misplaced. If the variable is null a NullPointerException will be thrown. Either the check is useless (the variable will never be ""null"") or it is incorrect"
public class Alpha {
void beta() {
if (b.equals(theta) && b != null) {}
}
}
public class Alpha {
void beta() {
if (b == null && b.equals(theta)) {}
}
}
Override Both Equals And Hashcode
Description: Override both public boolean Object.equals(Object other), and public int Object.hashCode(), or override neither
public class Alpha {
public boolean equals(Object a) {
//someComparison
}
}
public class Alpha {
public boolean equals(Object b) {
// someComparison
}
public int hashCode() {
// return hash value
}
}
Return From Finally Block
Description: Avoid returning from a finally block, this can discard exceptions
public class Alpha {
public String beta() {
try {
throw new Exception( "Exception" );
} catch (Exception e) {
throw e;
} finally {
return "This";
}
}
}
public class Alpha {
public String beta() {
try {
throw new Exception( "Exception" );
} catch (Exception e) {
throw e;
} finally {
//doSomething
}
}
}
Unconditional If Statement
Description: "Do not use ""if"" statements whose conditionals are always true or always false"
public class Alpha {
public void close() {
if (false) {
//doSomething
}
}
}
public class Alpha {
public void close() {
//doSomething
}
}
Unnecessary Conversion Temporary
Description: Avoid the use temporary objects when converting primitives to Strings. Use the static conversion methods on the wrapper classes instead
public String convert(int a) {
String alpha = new Integer(a).toString();
}
public String convert(int a) {
return Integer.toString(a);
}
Unused Null Check In Equals
Description: After checking an object reference for null, you should invoke equals() on that object rather than passing it to another object’s equals() method
public class Alpha {
public String alpha1() { return "done";}
public String alpha2() { return null;}
public void method(String x) {
String y;
if (x!=null && alpha1().equals(x)) {
//doSomething
}
}
}
public class Alpha {
public String alpha1() { return "done";}
public String alpha2() { return null;}
public void method(String x) {
String y;
if (x!=null && alpha1().equals(y)) {
//doSomething
}
}
}
Useless Operation On Immutable
Description: An operation on an Immutable object (String, BigDecimal or BigInteger) won’t change the object itself since the result of the operation is a new object
class Alpha {
void alpha1() {
BigDecimal bd=new BigDecimal(20);
bd.add(new BigDecimal(4));
}
}
class Alpha {
void alpha1() {
BigDecimal bd=new BigDecimal(20);
bd = bd.add(new BigDecimal(4));
}
}
Useless Overriding Method
Description: The overriding method merely calls the same method defined in a superclass
public void alpha(String beta) {
super.alpha(beta);
}
public Long getId() {
return super.getId();
}
For Loops Must Use Braces
Description: For Loops Must Use Braces
for (int j = 0; j < 34; j++)
alpha();
for (int j = 0; j < 34; j++) {
alpha()
};
If Else Stmts Must Use Braces
Description: If Else Stmts Must Use Braces
if (alpha)
i = i + 1;
else
i = i - 1;
if (alpha) {
i = i + 1;
} else {
i = i - 1;
}
If Stmts Must Use Braces
Description: If Stmts Must Use Braces
if (alpha)
i++;
if (alpha) {
i++;
}
While Loops Must Use Braces
Description: While Loops Must Use Braces
while (true)
i++;
while (true) {
i++;
}
Clone Throws Clone Not Supported Exception
Description: The method clone() should throw a CloneNotSupportedException
public class Alpha implements Cloneable{
public Object clone() {
Alpha clone = (Alpha)super.clone();
return clone;
}
}
Proper Clone Implementation
Description: Object clone() should be implemented with super.clone()
class Alpha{
public Object clone(){
return new Alpha();
}
}
Assignment In Operand
Description: Avoid assignments in operands. This can make code more complicated and harder to read
public void beta() {
int a = 2;
if ((a = getA()) == 3) {
System.out.println("3!");
}
}
Avoid Accessibility Alteration
Description: Methods such as getDeclaredConstructors(), getDeclaredConstructor(Class[]) and setAccessible(), as the interface PrivilegedAction, allow for the runtime alteration of variable, class, or method visibility, even if they are private. This violates the principle of encapsulation
public class Violation {
private void invalidSetAccessCalls() throws NoSuchMethodException, SecurityException {
Constructor<?> constructor = this.getClass().getDeclaredConstructor(String.class);
constructor.setAccessible(true);
Method privateMethod = this.getClass().getDeclaredMethod("aPrivateMethod");
privateMethod.setAccessible(true);
}
}
Avoid Prefixing Method Parameters
Description: Prefixing parameters by ‘in’ or ‘out’ pollutes the name of the parameters and reduces code readability
public class Alpha {
public void beta(
int inLeftOperand,
Result outRightOperand) {
outRightOperand.setValue(inLeftOperand * outRightOperand.getValue());
}
}
public class Alpha {
public void beta(
int leftOperand,
Result rightOperand) {
rightOperand.setValue(leftOperand * rightOperand.getValue());
}
}
Avoid Using Native Code
Description: Unnecessary reliance on Java Native Interface (JNI) calls directly reduces application portability and increases the maintenance burden
public class SomeNativeClass {
public SomeNativeClass() {
System.loadLibrary("nativelib");
}
static {
System.loadLibrary("nativelib");
}
public void invalidCallsInMethod() throws SecurityException, NoSuchMethodException {
System.loadLibrary("nativelib");
}
}
Default Package
Description: Use explicit scoping instead of accidental usage of default package private level
File saveFile = new File("C:/Upload/");
private File saveFile = new File("C:/Upload/");
Do Not Call Garbage Collection Explicitly
Description: Calls to System.gc(), Runtime.getRuntime().gc(), and System.runFinalization() are not advised. Code should have the same behavior whether the garbage collection is disabled using the option -Xdisableexplicitgc or not
public class AlphaGC {
public explicitAlphaGC() {
// Explicit garbage collector call
System.gc();
}
}
Dont Import Sun
Description: Avoid importing anything from the ‘sun.*’ packages. These packages are not portable and are likely to change
import sun.misc.bar;
One Declaration Per Line
Description: Java allows the use of several variables declaration of the same type on one line. However, it can lead to quite messy code
String first, last;
String first;
String last;
Suspicious Octal Escape
Description: A suspicious octal escape sequence was found inside a String literal
public void beta() {
System.out.println("suspicious: \128");
}
Unnecessary Constructor
Description: When there is only one constructor and the constructor is identical to the default constructor, then it is not necessary
public class Alpha {
public Alpha() {}
}
public class Alpha {
public Beta() {}
}
Abstract Class Without Abstract Method
Description: The abstract class does not contain any abstract methods. An abstract class suggests an incomplete implementation, which is to be completed by subclasses implementing the abstract methods
public abstract class Alpha {
void int method1() { ... }
void int method2() { ... }
}
Abstract Class Without Any Method
Description: If an abstract class does not provides any methods, it may be acting as a simple data container that is not meant to be instantiated
public abstract class Alpha {
String field;
int otherField;
}
Assignment To Non Final Static
Description: Possible unsafe usage of a static field
public class StaticField {
static int a;
public FinalFields(int b) {
a = b;
}
}
Avoid Constants Interface
Description: Avoid constants in interfaces. Interfaces should define types, constants are implementation details better placed in classes or enums
public interface AlphaInterface {
public static final int CONST1 = 1;
static final int CONST2 = 1;
final int CONST3 = 1;
int CONST4 = 1;
}
public interface BetaInterface {
public static final int CONST1 = 1;
int anyMethod();
}
Avoid Instanceof Checks In Catch Clause
Description: Each caught exception type should be handled in its own catch clause
try {
//doSomething
} catch (Exception ee) {
if (ee instanceof IOException) {
cleanup();
}
}
try {
//doSomething
} catch (IOException ee) {
cleanup();
}
Avoid Protected Field In Final Class
Description: Do not use protected fields in final classes since they cannot be subclassed
public final class Alpha {
private int a;
protected int b;
Alpha() {}
}
public final class Alpha {
private int a;
private int b;
Alpha() {}
}
Avoid Protected Method In Final Class Not Extending
Description: Do not use protected methods in most final classes since they cannot be subclassed
public final class Alpha {
private int beta() {}
protected int beta() {}
}
public final class Alpha {
private int beta() {}
private int beta() {}
}
Avoid Reassigning Parameters
Description: Reassigning values to incoming parameters is not recommended. Use temporary local variables instead
public class Boo {
private void boo(String tab) {
tab = "changed string";
}
}
public class Boo {
private void foo(String tab) {
String tab2 = String.join("A local value of tab: ", tab);;
}
}
Avoid Synchronized At Method Level
Description: Method-level synchronization can cause problems when new code is added to the method. Block-level synchronization helps to ensure that only the code that needs synchronization gets it
public class Alpha {
synchronized void alpha() {
}
}
public class Alpha {
void beta() {
synchronized(this) {
}
}
Bad Comparison
Description: BadComparison
boolean x = (y == Double.NaN);
Class With Only Private Constructors Should Be Final
Description: Avoid equality comparisons with Double.NaN due to the implicit lack of representation precision
Close Resource
Description: Ensure that resources (like java.sql.Connection, java.sql.Statement, and java.sql.ResultSet objects and any subtype of java.lang.AutoCloseable) are always closed after use. Failing to do so might result in resource leaks
public class Beta {
public void alpha() {
Connection a = pool.getConnection();
try {
// doSomething
} catch (SQLException ex) {
//exception
} finally {
//forgotToClose
}
}
}
public class Beta {
public void alpha() {
Connection a = pool.getConnection();
try {
// doSomething
} catch (SQLException ex) {
//exception
} finally {
a.close();
}
}
}
Constructor Calls Overridable Method
Description: Calling overridable methods during construction poses a risk of invoking methods on an incompletely constructed object and can be difficult to debug
public class SeniorClass {
public SeniorClass() {
toString();
}
public String toString(){
return "ThatSeniorClass";
}
}
public class JuniorClass extends SeniorClass {
private String name;
public JuniorClass() {
super();
name = "JuniorClass";
}
public String toString(){
return name.toUpperCase();
}
}
Default Label Not Last In Switch Stmt
Description: By convention, the default label should be the last label in a switch statement
public class Alpha {
void beta(int x) {
switch (x) {
case 1: // doSomething
break;
default:
break;
case 2:
break;
}
}
}
public class Alpha {
void beta(int x) {
switch (x) {
case 1: // doSomething
break;
case 2:
break;
default:
break;
}
}
}
Empty Method In Abstract Class Should Be Abstract
Description: Empty or auto-generated methods in an abstract class should be tagged as abstract
public abstract class NeedsToBeAbstract {
public Object mayBeAbstract() {
return null;
}
}
public abstract class NeedsToBeAbstract {
public void mayBeAbstract() {
}
}
Equals Null
Description: Tests for null should not use the equals() method. The ‘==’ operator should be used instead
String a = "alpha";
if (a.equals(null)) {
doSomething();
}
String a = "alpha";
if (a == null) {
doSomething();
}
Field Declarations Should Be At Start Of Class
Description: Fields should be declared at the top of the class, before any method declarations, constructors, initializers or inner classes
public class Alpha {
public String getMessage() {
return "Hello";
}
private String _something;
}
public class Alpha {
private String _fieldSomething;
public String getMessage() {
return "Hello";
}
}
Final Field Could Be Static
Description: If a final field is assigned to a compile-time constant, it could be made static, thus saving overhead in each object at runtime
public class Alpha {
public final int BETA = 18;
}
public class Alpha {
public static final int BETA = 18;
}
Idempotent Operations
Description: Avoid idempotent operations - they have no effect
public class Alpha {
public void beta() {
int a = 10;
a = a;
}
}
public class Alpha {
public void beta() {
int a = 10;
}
}
Immutable Field
Description: Private fields whose values never change once object initialization ends either in the declaration of the field or by a constructor should be final
public class Alpha {
private int x; // could be final
public Alpha() {
x = 7;
}
public void alpha() {
int a = x + 2;
}
}
Instantiation To Get Class
Description: Avoid instantiating an object just to call getClass() on it use the .class public member instead
Class a = new String().getClass();
Class a = String.class;
Logic Inversion
Description: Use opposite operator instead of negating the whole expression with a logic complement operator
public boolean beta(int x, int y) {
if (!(x == y)) {
return false;
}
return true;
}
public boolean beta(int x, int y) {
if (x != y) {
return false;
}
return true;
}
Missing Break In Switch
Description: Switch statements without break or return statements for each case option may indicate problematic behaviour. Empty cases are ignored as these indicate an intentional fall-through
public void alpha(int status) {
switch(status) {
case CANCELLED:
doSomething();
// break;
case OTHER:
case ERROR:
doSomethingElse();
break;
}
}
public void alpha(int status) {
switch(status) {
case CANCELLED:
doSomething();
break;
case ERROR:
doSomethingElse();
break;
}
}
Missing Static Method In Non Instantiatable Class
Description: A class that has private constructors and does not have any static methods or fields cannot be used
public class Alpha {
private Alpha() {}
void Alpha() {}
}
public class Alpha {
public static void main(String[] args) {
doSomething
}
}
Non Case Label In Switch Statement
Description: A non-case label (e.g. a named break/continue label) was present in a switch statement. This legal, but confusing. It is easy to mix up the case labels and the non-case labels
public class Alpha {
void beta(int x) {
switch (x) {
case 1:
doSomething;
break;
somelabel:
break;
default:
break;
}
}
}
Non Static Initializer
Description: A non-static initializer block will be called any time a constructor is invoked (just prior to invoking the constructor)
public class Alpha {
{
System.out.println("Construct");
}
}
Non Thread Safe Singleton
Description: Non-thread safe singletons can result in bad state changes. Eliminate static singletons if possible by instantiating the object directly. Static singletons are usually not needed as only a single instance exists anyway
private static Alpha alpha = null;
public static Alpha getAlpha() {
if (alpha == null) {
alpha = new Alpha();
}
return alpha;
}
Optimizable To Array Call
Description: Calls to a collection’s ‘toArray(E[])’ method should specify a target array of zero size
Alpha[] alphaArray = alphas.toArray(new Alpha[alphas.size()]);
Alpha[] alphaArray = alphas.toArray(new Alpha[0]);
Position Literals First In Case Insensitive Comparisons
Description: Position literals first in comparisons, if the second argument is null then NullPointerExceptions can be avoided, they will just return false
class Alpha {
boolean beta(String a) {
return a.equalsIgnoreCase("2");
}
}
class Alpha {
boolean beta(String a) {
return "2".equalsIgnoreCase(a);
}
}
Position Literals First In Comparisons
Description: Position literals first in comparisons, if the second argument is null then NullPointerExceptions can be avoided, they will just return false
class Alpha {
boolean beta(String a) {
return a.equals("2");
}
}
class Alpha {
boolean beta(String a) {
return "2".equals(a);
}
}
Preserve Stack Trace
Description: Throwing a new exception from a catch block without passing the original exception into the new exception will cause the original stack trace to be lost making it difficult to debug effectively
public class Alpha {
void beta() {
try{
Integer.parseInt("x");
} catch (Exception e) {
throw new Exception(e.getMessage());
}
}
}
public class Alpha {
void beta() {
try{
Integer.parseInt("x");
} catch (Exception e) {
throw new Exception(e);
}
try {
Integer.parseInt("x");
} catch (Exception e) {
throw (IllegalStateException)new IllegalStateException().initCause(e);
}
}
}
Return Empty Array Rather Than Null
Description: For any method that returns an array, it is a better to return an empty array rather than a null reference. This removes the need for null checking all results and avoids inadvertent NullPointerExceptions
public class Alpha {
public int[] beta() {
//doSomething
return null;
}
}
public class Alpha {
public String[] beta() {
//doSomething
return new String[0];
}
}
Simple Date Format Needs Locale
Description: Be sure to specify a Locale when creating SimpleDateFormat instances to ensure that locale-appropriate formatting is used
public class Alpha {
// Should specify Locale.US (or whatever)
private SimpleDateFormat sdf = new SimpleDateFormat("pattern");
}
Simplify Boolean Expressions
Description: Avoid unnecessary comparisons in boolean expressions, they serve no purpose and impacts readability
public class Beta {
private boolean beta = (isAlpha() == true);
public isAlpha() { return false;}
}
public class Beta {
beta = isAlpha();
public isAlpha() { return false;}
}
Simplify Boolean Returns
Description: Avoid unnecessary if-then-else statements when returning a boolean. The result of the conditional test can be returned instead
public boolean isBetaEqualTo(int a) {
if (beta == a) {
return true;
} else {
return false;
}
}
public boolean isBetaEqualTo(int a) {
return beta == a;
}
Simplify Conditional
Description: No need to check for null before an instanceof. The instanceof keyword returns false when given a null argument
class Alpha {
void beta(Object a) {
if (a != null && a instanceof Beta) {
//doSomething
}
}
}
class Alpha {
void beta(Object a) {
if (a instanceof Beta) {
//doSomething
}
}
}
Singular Field
Description: Fields whose scopes are limited to just single methods do not rely on the containing object to provide them to other methods. They may be better implemented as local variables within those methods
public class Alpha {
private int a;
public void alpha(int b) {
a = b + 2;
return a;
}
}
public class Alpha {
public void alpha(int b) {
a = b + 2;
return a;
}
}
Switch Stmts Should Have Default
Description: All switch statements should include a default option to catch any unspecified values
public void beta() {
int a = 5;
switch (a) {
case 1: int b = 2;
case 2: int b = 4;
}
}
public void beta() {
int a = 5;
switch (a) {
case 1: int b = 2;
case 2: int b = 4;
default: break;
}
}
Too Few Branches For ASwitch Statement
Description: Switch statements are intended to be used to support complex branching behaviour. Using a switch for only a few cases is ill-advised, since switches are not as easy to understand as if-then statements
public class Alpha {
public void beta() {
switch (a) {
case 1:
doSomething;
break;
default:
break;
}
}
}
public class Alpha {
public void beta() {
if (something) {
doSomething;
}
}
}
Uncommented Empty Constructor
Description: By explicitly commenting empty constructors it is easier to distinguish between intentional (commented) and unintentional empty constructors
public Alpha() {
}
public Alpha() {
//somethingCommented
}
Uncommented Empty Method
Description: By explicitly commenting empty method bodies it is easier to distinguish between intentional (commented) and unintentional empty methods
public void alpha() {
}
public void alpha() {
//somethingCommented
}
Unnecessary Local Before Return
Description: Avoid the creation of unnecessary local variables
public class Alpha {
public int alpha() {
int a = doSomething();
return a;
}
}
public class Alpha {
public int alpha() {
return doSomething();
}
}
Unsynchronized Static Date Formatter
Description: SimpleDateFormat instances are not synchronized. Sun recommends using separate format instances for each thread. If multiple threads must access a static formatter, the formatter must be synchronized on block leve
public class Alpha {
private static final SimpleDateFormat sdf = new SimpleDateFormat();
void beta() {
sdf.format();
}
}
public class Alpha {
private static final SimpleDateFormat sdf = new SimpleDateFormat();
synchronized void alpha() {
sdf.format();
}
}
Use Collection Is Empty
Description: The isEmpty() method on java.util.Collection is provided to determine if a collection has any elements. Comparing the value of size() to 0 does not convey intent as well as the isEmpty() method
public class Alpha {
void beta() {
List alpha = getList();
if (alpha.size() == 0) {
//doSomething
}
}
}
public class Alpha {
void beta() {
List alpha = getList();
if (alpha.isEmpty()) {
//doSomething
}
}
}
Use Locale With Case Conversions
Description: When doing String::toLowerCase()/toUpperCase() conversions, use an explicit locale argument to specify the case transformation rules
class Alpha {
if (a.toLowerCase().equals("list")) { }
}
class Alpha {
String x = a.toLowerCase(Locale.EN);
}
Use Notify All Instead Of Notify
Description: Thread.notify() awakens a thread monitoring the object. If more than one thread is monitoring, then only one is chosen. The thread chosen is arbitrary and thus its usually safer to call notifyAll() instead
void beta() {
a.notify();
}
void beta() {
a.notifyAll();
}
Use Varargs
Description: Java 5 introduced the varargs parameter declaration for methods and constructors. This syntactic sugar provides flexibility for users of these methods and constructors, allowing them to avoid having to deal with the creation of an array
public class Alpha {
public void alpha(String a, Object[] args) {
//doSomething
}
}
public class Alpha {
public void alpha(String a, Object... args) {
//doSomething
}
}
Avoid Calling Finalize
Description: The method Object.finalize() is called by the garbage collector on an object when garbage collection determines that there are no more references to the object. It should not be invoked by application logic
void alpha() {
Beta a = new Beta();
a.finalize();
}
Empty Finalizer
Description: Empty finalize methods serve no purpose and should be removed. Note that Oracle has declared Object.finalize() as deprecated since JDK 9
public class Alpha {
protected void finalize() {}
}
Finalize Does Not Call Super Finalize
Description: If the finalize() is implemented, its last action should be to call super.finalize. Note that Oracle has declared Object.finalize() as deprecated since JDK 9
protected void finalize() {
doSomething();
}
protected void finalize() {
doSomething();
super.finalize();
}
Finalize Only Calls Super Finalize
Description: If the finalize() is implemented, it should do something besides just calling super.finalize(). Note that Oracle has declared Object.finalize() as deprecated since JDK 9
protected void finalize() {
super.finalize();
}
protected void finalize() {
doSomething();
super.finalize();
}
Finalize Overloaded
Description: Methods named finalize() should not have parameters. It is confusing and most likely an attempt to overload Object.finalize(). It will not be called by the VM
public class Alpha {
protected void finalize(int x) {
}
}
Finalize Should Be Protected
Description: When overriding the finalize(), the new method should be set as protected. If made public, other classes may invoke it at inappropriate times
public void finalize() {
//doSomething
}
protected void finalize() {
//doSomething
}
Dont Import Java Lang
Description: Avoid importing anything from the package ‘java.lang’. These classes are automatically imported
import java.lang.String;
Duplicate Imports
Description: Duplicate or overlapping import statements should be avoided
import java.lang.String;
import java.lang.*;
import java.lang.*;
Import From Same Package
Description: There is no need to import a type that lives in the same package
package alpha;
import alpha.Beta;
package alpha;
Too Many Static Imports
Description: If you overuse the static import feature, it can make your program unreadable and unmaintainable, polluting its namespace with all the static members you import
import static Alpha;
import static Beta;
import static Theta;
import static Omicron;
Unnecessary Fully Qualified Name
Description: Import statements allow the use of non-fully qualified names. The use of a fully qualified name which is covered by an import statement is redundant
public class Alpha {
private java.util.List list1;
private List list2;
}
Do Not Call System Exit
Description: Web applications should not call System.exit(), since only the web container or the application server should stop the JVM
public void beta() {
System.exit(0);
}
Local Home Naming Convention
Description: The Local Home interface of a Session EJB should be suffixed by ‘LocalHome’
public interface MissingProperSuffix extends javax.ejb.EJBLocalHome {}
public interface MyBeautifulLocalHome extends javax.ejb.EJBLocalHome {}
Local Interface Session Naming Convention
Description: The Local Interface of a Session EJB should be suffixed by ‘Local’
public interface MissingProperSuffix extends javax.ejb.EJBLocalObject {}
public interface MyLocal extends javax.ejb.EJBLocalObject {}
MDBAnd Session Bean Naming Convention
Description: The EJB Specification states that any MessageDrivenBean or SessionBean should be suffixed by ‘Bean’
public class MissingTheProperSuffix implements SessionBean {}
public class SomeBean implements SessionBean{}
Remote Interface Naming Convention
Description: Remote Interface of a Session EJB should not have a suffix
public interface BadSuffixSession extends javax.ejb.EJBObject {}
Remote Session Interface Naming Convention
Description: A Remote Home interface type of a Session EJB should be suffixed by ‘Home’
public interface MissingProperSuffix extends javax.ejb.EJBHome {}
public interface MyHome extends javax.ejb.EJBHome {}
Static EJBField Should Be Final
Description: According to the J2EE specification, an EJB should not have any static fields with write access. However, static read-only fields are allowed
public class SomeEJB extends EJBObject implements EJBLocalHome {
private static int CountB;
}
public class SomeEJB extends EJBObject implements EJBLocalHome {
private static final int CountB;
}
JUnit Assertions Should Include Message
Description: JUnit assertions should include an informative message - i.e., use the three-argument version of assertEquals(), not the two-argument version
public class Alpha extends Beta {
public void theta() {
assertEquals("alpha", "beta");
}
}
public class Alpha extends Beta {
public void theta() {
assertEquals("Alpha does not equals beta", "alpha", "beta");
}
}
JUnit Spelling
Description: Some JUnit framework methods are easy to misspell
import junit.framework.*;
public class Alpha extends Beta {
public void setup() {}
}
import junit.framework.*;
public class Alpha extends Beta {
public void setUp() {}
}
JUnit Static Suite
Description: The suite() method in a JUnit test needs to be both public and static
import junit.framework.*;
public class Alpha extends Beta {
public void suite() {}
}
import junit.framework.*;
public class Alpha extends Beta {
public static void suite() {}
}
JUnit Test Contains Too Many Asserts
Description: Unit tests should not contain too many asserts. Many asserts are indicative of a complex test, for which it is harder to verify correctness
public class Alpha extends Beta {
public void testAlpha() {
boolean myTheta = false;
assertFalse("myTheta should be false", myTheta);
assertEquals("should equals false", false, myTheta);
}
}
public class Alpha extends Beta {
public void testAlpha() {
boolean myTheta = false;
assertFalse("should be false", myTheta);
}
}
JUnit Tests Should Include Assert
Description: JUnit tests should include at least one assertion. This makes the tests more robust, and using assert with messages provide the developer a clearer idea of what the test does
public class Foo extends TestCase {
public void testSomething() {
Bar b = findBar();
b.work();
}
}
public class Foo extends TestCase {
public void testSomething() {
Bar b = findBar();
// This is better than having a NullPointerException
assertNotNull("bar not found", b);
}
}
Simplify Boolean Assertion
Description: Avoid negation in an assertTrue or assertFalse test
assertTrue(!sth);
assertFalse(sth);
Test Class Without Test Cases
Description: Test classes end with the suffix Test. Having a non-test class with that name is not a good practice, since most people will assume it is a test case
public class CarTest {
public static void main(String[] args) {
}
}
Unnecessary Boolean Assertion
Description: A JUnit test assertion with a boolean literal is unnecessary since it always will evaluate to the same thing. Consider using flow control (in case of assertTrue(false) or similar) or simply removing statements like assertTrue(true) and assertFalse(false)
public class Alpha extends Beta {
public void testAlpha() {
assertTrue(true);
}
}
Use Assert Equals Instead Of Assert True
Description: The assertions should be made by more specific methods, like assertEquals
public class Alpha extends Beta {
void testAlpha() {
Object x, y;
assertTrue(x.equals(y));
}
}
public class Alpha extends Beta {
void testAlpha() {
Object x, y;
assertEquals("x should equals y", x, y);
}
}
Use Assert Null Instead Of Assert True
Description: The assertions should be made by more specific methods, like assertNull, assertNotNull
public class Alpha extends Beta {
void testAlpha() {
Object x = doSomething();
assertTrue(x == null);
}
}
public class Alpha extends Beta {
void testAlpha() {
Object x = doSomething();
assertNull(x);
}
}
Use Assert Same Instead Of Assert True
Description: The assertions should be made by more specific methods, like assertSame, assertNotSame
public class Alpha extends Beta {
void testAlpha() {
Object x, y;
assertTrue(x == y);
}
}
public class Alpha extends Beta {
void testAlpha() {
Object x, y;
assertSame(x, y);
}
}
Use Assert True Instead Of Assert Equals
Description: When asserting a value is the same as a literal or Boxed boolean, use assertTrue/assertFalse, instead of assertEquals
public class Alpha extends Beta {
public void testAlpha() {
boolean myTest = true;
assertEquals("myTest is true", true, myTest);
}
}
public class Alpha extends Beta {
public void testAlpha() {
boolean myTest = true;
assertTrue("myTest is true", myTest);
}
}
Guard Debug Logging
Description: When log messages are composed by concatenating strings, the whole section should be guarded by a isDebugEnabled() check to avoid performance and memory issues
logger.debug("This is a very long message that prints two values." +
" However since the message is long, we still incur the performance hit" +
" of String concatenation when logging {} and {}", value1, value2);
if (logger.isDebugEnabled()) {
logger.debug("This is a very long message that prints two values." +
" However since the message is long, we still incur the performance hit" +
" of String concatenation when logging {} and {}", value1, value2);
}
Guard Log Statement
Description: Whenever using a log level, one should check if the loglevel is actually enabled, or otherwise skip the associate String creation and manipulation
log.debug("logs here {} and {}", param1, param2);
if (log.isDebugEnabled()) {
log.debug("logs here" + param1 + " and " + param2 + "concat strings");
}
Proper Logger
Description: A logger should normally be defined private static final and be associated with the correct class
public class Alpha {
protected Log LOG = LogFactory.getLog(Testalpha.class);
}
public class Alpha {
private static final Log LOG = LogFactory.getLog(Alpha.class);
}
Use Correct Exception Logging
Description: To make sure the full stacktrace is printed out, use the logging statement with two arguments: a String and a Throwable
public class Alpha {
private static final Log _LOG = LogFactory.getLog( Alpha.class );
void beta() {
try {
} catch( Exception e ) {
_LOG.error( e );
}
}
}
public class Alpha {
private static final Log _LOG = LogFactory.getLog( Alpha.class );
void beta() {
try {
} catch( OtherException oe ) {
_LOG.error( oe.getMessage(), oe );
}
}
}
Avoid Print Stack Trace
Description: Avoid printStackTrace() use a logger call instead
class Alpha {
void beta() {
try {
//doSomething
} catch (Exception e) {
e.printStackTrace();
}
}
}
Guard Log Statement Java Util
Description: Whenever using a log level, one should check if the loglevel is actually enabled, or otherwise skip the associate String creation and manipulation
log.debug("log something {} and {}", param1, param2);
if (log.isDebugEnabled()) {
log.debug("log something" + param1 + " and " + param2 + "concat strings");
}
Logger Is Not Static Final
Description: In most cases, the Logger reference can be declared as static and final
public class Alpha{
Logger log = Logger.getLogger(Alpha.class.getName());
}
public class Alpha{
static final Logger log = Logger.getLogger(Alpha.class.getName());
}
More Than One Logger
Description: Normally only one logger is used in each class
public class Alpha {
Logger log1 = Logger.getLogger(Alpha.class.getName());
Logger log2= Logger.getLogger(Alpha.class.getName());
}
public class Alpha {
Logger log = Logger.getLogger(Alpha.class.getName());
}
System Println
Description: References to System.(out|err).print are usually intended for debugging purposes. Use a logger instead
class Alpha{
Logger log = Logger.getLogger(Alpha.class.getName());
public void testAlpha () {
System.out.println("This test");
}
}
class Alpha{
Logger log = Logger.getLogger(Alpha.class.getName());
public void testAlpha () {
log.fine("This test");
}
}
Missing Serial Version UID
Description: Serializable classes should provide a serialVersionUID field
public class Alpha implements java.io.Serializable {
String test;
//doSomething
}
public class Alpha implements java.io.Serializable {
String test;
public static final long serialVersionUID = 4916737;
}
Avoid Dollar Signs
Description: Avoid using dollar signs in variable/method/class/interface names
public class Alp$ha {
}
public class Alpha {
}
Avoid Field Name Matching Method Name
Description: It can be confusing to have a field name with the same name as a method
public class Alpha {
Object beta;
void beta() {
}
}
public class Alpha {
Object beta;
void theta() {
}
}
Avoid Field Name Matching Type Name
Description: It is somewhat confusing to have a field name matching the declaring class name
public class Alpha extends Beta {
int alpha;
}
public class Alpha extends Beta {
int theta;
}
Boolean Get Method Name
Description: Methods that return boolean results should be named as predicate statements to denote this. I.e, ‘isReady()’, ‘hasValues()’, ‘canCommit()’, ‘willFail()’, etc. Avoid the use of the ‘get’ prefix for these methods
public boolean getAlpha();
public boolean isAlpha();
Class Naming Conventions
Description: Configurable naming conventions for type declarations
public class Étudiant {}
public class AlphaBeta {}
Generics Naming
Description: Names for references to generic values should be limited to a single uppercase letter
public interface GenericDao<e extends BaseModel, K extends Serializable> {
}
public interface GenericDao<E extends BaseModel, K extends Serializable> {
}
Method Naming Conventions
Description: Configurable naming conventions for method declarations
Method With Same Name As Enclosing Class
Description: Non-constructor methods should not have the same name as the enclosing class
public class AlphaT {
public void AlphaT() {}
}
public class AlphaT {
public AlphaT() {}
}
No Package
Description: A class, interface, enum or annotation does not have a package definition
Package Case
Description: The package definition contains uppercase characters
package com.MyAlpha;
package com.myalpha;
Short Class Name
Description: Short Classnames with fewer than e.g. five characters are not recommended
public class Alp {
}
public class Alpha {
}
Short Method Name
Description: Method names that are very short are not helpful to the reader
public class Alpha {
public void a( int i ) {
}
}
public class Alpha {
public void beta( int i ) {
}
}
Suspicious Constant Field Name
Description: Field names using all uppercase characters - Sun’s Java naming conventions indicating constants - should be declared as final
public class Alpha {
double PI = 3.16;
}
public class Alpha {
final double PI = 3.16;
}
Suspicious Equals Method Name
Description: The method name and parameter number are suspiciously close to equals(Object), which can denote an intention to override the equals(Object) method
public class Alpha {
public int equals(Object a) {
//doSomething
}
}
public class Alpha {
public boolean equals(Object a) {
//doSomething
}
}
Suspicious Hashcode Method Name
Description: The method name and return type are suspiciously close to hashCode(), which may denote an intention to override the hashCode() method
public class Alpha {
public int hashcode() {
}
}
public class Alpha {
public int newname() {
}
}
Variable Naming Conventions
Description: Final variables should be fully capitalized and non-final variables should not include underscores
public class Alpha {
public static final int my_alp = 0;
}
public class Alpha {
public static final int MY_ALP = 0;
}
Add Empty String
Description: The conversion of literals to strings by concatenating them with empty strings is inefficient. It is much better to use one of the type-specific toString() methods instead
String a = "" + 456;
String a = Integer.toString(456);
Avoid Array Loops
Description: Instead of manually copying data between two arrays, use the efficient Arrays.copyOf or System.arraycopy method instead
public class Alpha {
public void beta() {
int[] x = new int[5];
int[]y = new int[5];
for (int i = 0; i < 5 ; i++) {
y[i] = x[i];
}
int[] z = new int[5];
for (int i = 0 ; i < 5 ; i++) {
y[i] = x[z[i]];
}
}
}
Redundant Field Initializer
Description: Java will initialize fields with known default values so any explicit initialization of those same defaults is redundant and results in a larger class file (approximately three additional bytecode instructions per field)
public class Alpha {
boolean a = false;
}
Unnecessary Wrapper Object Creation
Description: Most wrapper classes provide static conversion methods that avoid the need to create intermediate objects just to create the primitive forms. Using these avoids the cost of creating objects that also need to be garbage-collected later
public int convert(String a) {
int x, x2;
x = Integer.valueOf(a).intValue();
x2 = Integer.valueOf(x).intValue();
return x2;
}
public int convert(String a) {
int x, x2;
x = Integer.parseInt(a);
x2 = x;
return x2;
}
Use Array List Instead Of Vector
Description: ArrayList is a much better Collection implementation than Vector if thread-safe operation is not required
public class Alpha extends Beta {
public void testAlpha() {
Collection b = new Vector();
}
}
public class Alpha extends Beta {
public void testAlpha() {
Collection b = new ArrayList();
}
}
Use Arrays As List
Description: "The java.util.Arrays class has a ""asList"" method that should be used when you want to create a new List from an array of objects. It is faster than executing a loop to copy all the elements of the array one by one"
public class Alpha {
public void beta(Integer[] ints) {
List<Integer> l = new ArrayList<>(50);
for (int i = 0 ; i < 50; i++) {
l.add(ints[i]);
}
}
}
public class Alpha {
public void beta(Integer[] ints) {
List<Integer> l= new ArrayList<>(50);
for (int i = 0 ; i < 50; i++) {
l.add(a[i].toString());
}
}
}
Use String Buffer For String Appends
Description: The use of the ‘+=’ operator for appending strings causes the JVM to create and use an internal StringBuffer. If a non-trivial number of these concatenations are being used then the explicit use of a StringBuilder or threadsafe StringBuffer is recommended to avoid this
public class Alpha {
void beta() {
String c;
c = "alpha";
c += " beta";
}
}
public class Alpha {
void beta() {
String c;
StringBuilder c = new StringBuilder("alpha");
c.append(" beta");
}
}
Array Is Stored Directly
Description: Constructors and methods receiving arrays should clone objects and store the copy. This prevents future changes from the user from affecting the original array
public class Alpha {
private String [] a;
public void beta (String [] param) {
this.a=param;
}
}
Method Returns Internal Array
Description: Exposing internal arrays to the caller violates object encapsulation since elements can be removed or replaced outside of the object that owns it. It is safer to return a copy of the array
public class Alpha {
UserChart [] uc;
public UserChart [] getUserChart() {
return uc;
}
}
Avoid Catching Generic Exception
Description: Avoid catching generic exceptions such as NullPointerException, RuntimeException, Exception in try-catch block
public class Alpha {
public void Beta() {
try {
System.out.println(" i [" + i + "]");
} catch(Exception e) {
e.printStackTrace();
} catch(RuntimeException e) {
e.printStackTrace();
} catch(NullPointerException e) {
e.printStackTrace();
}
}
}
Avoid Catching NPE
Description: Code should never throw NullPointerExceptions under normal circumstances. A catch block may hide the original error, causing other, more subtle problems later on
public class Alpha {
void beta() {
try {
// do something
} catch (NullPointerException npe) {
}
}
}
Avoid Catching Throwable
Description: Catching Throwable errors is not recommended since its scope is very broad. It includes runtime issues such as OutOfMemoryError that should be exposed and managed separately
public void beta() {
try {
// do something
} catch (Throwable th) {
th.printStackTrace();
}
}
Avoid Losing Exception Information
Description: Statements in a catch block that invoke accessors on the exception without using the information only add to code size. Either remove the invocation, or use the return result
public void bar() {
try {
// do something
} catch (SomeException se) {
se.getMessage();
}
}
Avoid Rethrowing Exception
Description: Catch blocks that merely rethrow a caught exception only add to code size and runtime complexity
public void beta() {
try {
// do something
} catch (SomeException se) {
throw se;
}
}
Avoid Throwing New Instance Of Same Exception
Description: Catch blocks that merely rethrow a caught exception wrapped inside a new instance of the same type only add to code size and runtime complexity
public void beta() {
try {
// do something
} catch (SomeException se) {
// harmless comment
throw new SomeException(se);
}
}
Avoid Throwing Null Pointer Exception
Description: Avoid throwing NullPointerExceptions manually. These are confusing because most people will assume that the virtual machine threw it. To avoid a method being called with a null parameter, you may consider using an IllegalArgumentException instead
public class Alpha {
void beta() {
throw new NullPointerException();
}
}
public class Alpha {
private String examValue;
void setExamValue(String examValue) {
this.examValue = Objects.requireNonNull(examValue, "examValue must not be null!");
}
}
Avoid Throwing Raw Exception Types
Description: Avoid throwing certain exception types. Rather than throw a raw RuntimeException, Throwable, Exception, or Error, use a subclassed exception or error instead
public class Alpha {
public void beta() throws Exception {
throw new Exception();
}
}
Do Not Extend Java Lang Error
Description: Errors are system exceptions. Do not extend them
public class Alpha extends Error { }
Do Not Throw Exception In Finally
Description: "Throwing exceptions within a ‘finally’ block is confusing since they may mask other exceptions or code defects. Note: This is a PMD implementation of the Lint4j rule ""A throw in a finally block"""
public class Alpha {
public void beta() {
try {
// Do somthing
} catch( Exception e) {
// Handling the issue
} finally {
throw new Exception();
}
}
}
Exception As Flow Control
Description: Using Exceptions as form of flow control is not recommended as they obscure true exceptions when debugging. Either add the necessary validation or use an alternate control structure
public void beta() {
try {
try {
} catch (Exception e) {
throw new WrapperException(e);
}
} catch (WrapperException e) {
// do some more stuff
}
}
Avoid Duplicate Literals
Description: Code containing duplicate String literals can usually be improved by declaring the String as a constant field
private void alpha() {
beta("Howdy");
beta("Howdy");
}
private void beta(String x) {}
Avoid String Buffer Field
Description: StringBuffers/StringBuilders can grow considerably, and so may become a source of memory leaks if held within objects with long lifetimes
public class Foo {
private StringBuffer buffer;
}
Consecutive Appends Should Reuse
Description: Consecutive calls to StringBuffer/StringBuilder .append should be chained, reusing the target object. This can improve the performance by producing a smaller bytecode, reducing overhead and improving inlining
buf.append("Hello");
buf.append(alpha);
buf.append("World");
buf.append("Hello").append(alpha).append("World");
Consecutive Literal Appends
Description: Consecutively calling StringBuffer/StringBuilder.append(…) with literals should be avoided
buf.append("Hello").append(" ").append("World");
buf.append("Hello World");
Inefficient String Buffering
Description: Avoid concatenating non-literals in a StringBuffer constructor or append() since intermediate buffers will need to be be created and destroyed by the JVM
StringBuffer sb = new StringBuffer("tmp = "+System.getProperty("java.io.tmpdir"));
StringBuffer sb = new StringBuffer("tmp = ");
sb.append(System.getProperty("java.io.tmpdir"));
String Buffer Instantiation With Char
Description: Individual character values provided as initialization arguments will be converted into integers. This can lead to internal buffer sizes that are larger than expected
StringBuffer sb1 = new StringBuffer('c');
StringBuilder sb2 = new StringBuilder('c');
StringBuffer sb3 = new StringBuffer("c");
StringBuilder sb4 = new StringBuilder("c");
String Instantiation
Description: Avoid instantiating String objects. This is usually unnecessary since they are immutable and can be safely shared
private String beta = new String("beta");
private String beta = "beta";
String To String
Description: Avoid calling toString() on objects already known to be string instances. This is unnecessary
private String beta() {
String tab = "iamastring";
return tab.toString();
}
private String beta() {
String tab = "a string";
return String.join("I am ", tab);
}
Unnecessary Case Change
Description: Using equalsIgnoreCase() is faster than using toUpperCase/toLowerCase().equals()
boolean answer = alpha.toUpperCase().equals("beta");
boolean answer = alpha.equalsIgnoreCase("beta")
Use Equals To Compare Strings
Description: Using ‘==’ or ‘!=’ to compare strings only works if intern version is used on both sides
public boolean test(String s) {
if (s == "one") return true;
return false;
}
public boolean test(String s) {
if ("two".equals(s)) return true;
return false;
}
Clone Method Must Implement Cloneable
Description: The method clone() should only be implemented if the class implements the Cloneable interface with the exception of a final method that only throws CloneNotSupportedException
public class MyClass {
public Object clone() {
return alpha;
}
}
public class MyClass {
public Object clone() throws CloneNotSupportedException {
return alpha;
}
}
Loose Coupling
Description: The use of implementation types (i.e., HashSet) as object references limits your ability to use alternate implementations in the future as requirements change. Whenever available, referencing objects by their interface types (i.e, Set) provides much more flexibility
public class Beta {
private ArrayList<SomeType> list = new ArrayList<>();
public HashSet<SomeType> getAlpha() {
return new HashSet<SomeType>();
}
}
public class Beta {
private List<SomeType> list = new ArrayList<>();
public Set<SomeType> getAlpha() {
return new HashSet<SomeType>();
}
}
Signature Declare Throws Exception
Description: A method/constructor shouldn’t explicitly throw the generic java.lang.Exception, since it is unclear which exceptions that can be thrown from the methods. It might be difficult to document and understand such vague interfaces. Use either a class derived from RuntimeException or a checked exception
public void alpha() throws Exception { }
Unused Imports
Description: Avoid unused import statements to prevent unwanted dependencies
import java.io.File;
import java.util.*;
public class Alpha {}
public class ALpha {}
Unused Local Variable
Description: The local variable is declared and/or assigned, but not used
public class Alpha {
public void doSomething(Int x) {
int i = 5;
return x + 2;
}
}
public class Alpha {
public void doSomething(Int x) {
int i = 5;
public int addOne() {
return j + i;
}
}
}
Unused Private Field
Description: The private field is declared and/or assigned a value, but not used
public class Something {
private static int ALPHA = 2;
private int i = 5;
}
public class Something {
private int j = 6;
public int addOne() {
return j++;
}
}
Unused Private Method
Description: The private method is declared but is unused
public class Something {
private void alpha() {}
}
Accessor Method Generation
Description: Avoid autogenerated methods to access private fields and methods of inner/outer classes
public class OuterClass {
public class InnerClass {
InnerClass() {
OuterClass.this.counter++;
}
}
}
public class OuterClass {
private int counter;
}
Avoid Message Digest Field
Description: It is better to create a new instance, rather than synchronizing access to a shared instance
public byte[] calculateHashShared(byte[] data) {
sharedMd.reset();
sharedMd.update(data);
return sharedMd.digest();
}
public byte[] calculateHash(byte[] data) throws Exception {
MessageDigest md = MessageDigest.getInstance("SHA-256");
md.update(data);
return md.digest();
}
Avoid Reassigning Catch Variables
Description: Reassigning exception variables caught in a catch statement should be avoided
public class Alpha {
public void foo() {
try {
// do something
} catch (Exception e) {
e = new NullPointerException();
}
}
}
public class Alpha {
public void foo() {
try {
// do something
} catch (MyException | ServerException e) {
e = new RuntimeException();
}
}
}
Avoid Reassigning Loop Variables
Description: Reassigning loop variables can lead to hard-to-find bugs. Prevent or limit how these variables can be changed
public class Alpha {
private void alpha() {
for (String s : listOfStrings()) {
s = s.trim();
doSomethingWith(s);
s = s.toUpper();
doSomethingElseWith(s);
}
}
}
public class Alpha {
private voidalpha() {
for (int i=0; i < 10; i++) {
if (check(i)) {
i++;
}
i = 5;
doSomethingWith(i);
}
}
}
Constants In Interface
Description: Avoid constants in interfaces. Interfaces define types, constants are implementation details better placed in classes or enums
public interface ConstantInterface {
public static final int CONST1 = 1;
static final int CONST2 = 1;
final int CONST3 = 1;
int CONST4 = 1;
}
public interface ConstantInterface {
public static final int CONST1 = 1;
int anyMethod();
}
Double Brace Initialization
Description: it is preferable to initialize the object normally, rather than usuing double brace initialization
List<String> a = new ArrayList<>();
a.add("a");
a.add("b");
a.add("c");
return a;
return new ArrayList<String>(){{
add("a");
add("b");
add("c");
}};
For Loop Can Be Foreach
Description: Reports loops that can be safely replaced with the foreach syntax
public class Alpha {
void loop(List<String> l) {
for (int i = 0; i < l.size(); i++) {
System.out.println(l.get(i));
}
}
}
public class Alpha {
void loop(List<String> l) {
for (String s : l) {
System.out.println(s);
}
}
}
For Loop Variable Count
Description: Having a lot of control variables in a 'for' loop makes it harder to see what range of values the loop iterates over
for (int i = 0, j = 0; i < 10; i++, j += 2) {
alpha();
}
for (int i = 0; i < 10; i++) {
alpha();
}
Literals First In Comparisons
Description: Position literals first in all String comparisons
class Alpha {
boolean bar(String x) {
return x.equals("3");
}
boolean bar(String x) {
return x.equalsIgnoreCase("3");
}
boolean bar(String x) {
return (x.compareTo("alpha") > 0);
}
boolean bar(String x) {
return (x.compareToIgnoreCase("alpha") > 0);
}
boolean bar(String x) {
return x.contentEquals("alpha");
}
}
class Alpha {
boolean bar(String x) {
return "3".equals(x);
}
boolean bar(String x) {
return "3".equalsIgnoreCase(x);
}
boolean bar(String x) {
return ("alpha".compareTo(x) < 0);
}
boolean bar(String x) {
return ("alpha".compareToIgnoreCase(x) < 0);
}
boolean bar(String x) {
return "alpha".contentEquals(x);
}
}
Missing Override
Description: Annotating overridden methods with @Override helps refactoring and clarifies intent
public class Alpha implements Run {
public void run() {
}
}
public class Alpha implements Run {
@Override
public void run() {
}
}
Use Try With Resources
Description: Prefer usuing Try With Resources because ensures that each resource is closed at the end of the statement
public class Try {
public void run() {
InputStream in = null;
try {
in = openInputStream();
int i = in.read();
} catch (IOException e) {
e.printStackTrace();
} finally {
try {
if (in != null) in.close();
} catch (IOException ignored) {
// ignored
}
}
}
}
public class Try {
public void run() {
InputStream in = null;
// better use try-with-resources
try (InputStream in2 = openInputStream()) {
int i = in2.read();
}
}
}
While Loop With Literal Boolean
Description: While loops that include literal booleans are redundant and should be avoided
public class Alpha {
{
while (false) { } // disallowed
do { } while (true); // disallowed
do { } while (false); // disallowed
}
}
public class Example {
{
while (true) { } // allowed
}
}
Control Statement Braces
Description: Enforce a policy for braces on control statements
while (true)
x++;
while (true) {
x++;
}
Identical Catch Branches
Description: Prefer collapsing identical branches into a single multi-catch branch
try {
// do something
} catch (IllegalArgumentException e) {
throw e;
} catch (IllegalStateException e) {
throw e;
}
try {
// do something
} catch (IllegalArgumentException | IllegalStateException e) {
throw e;
}
Unnecessary Annotation Value Element
Description: Avoid the use of value in annotations when it's the only element
@TestClassAnnotation(value = "TEST")
public class Alpha {
@TestMemberAnnotation(value = "TEST")
private String alpha;
@TestMethodAnnotation(value = "TEST")
public void beta() {
int gamma = 42;
return;
}
}
@TestClassAnnotation("TEST")
public class Alpha {
@TestMemberAnnotation("TEST")
private String alpha;
@TestMethodAnnotation("TEST")
public void beta() {
int gamma = 42;
return;
}
}
Unnecessary Modifier
Description: There is no need to modify the interfaces and the annotations
public interface Alpha {
public abstract void alpha();
}
public interface Alpha {
void alpha();
}
Use Short Array Initializer
Description: Define the initial content of the array as a expression in curly braces
Alpha[] x = new Alpha[] { ... };
Alpha[] x = { ... };
Use Underscores In Numeric Literals
Description: This rule enforces that numeric literals above a certain length should separate every third digit with an underscore
public class Alpha {
private int num = 1000000;
}
public class Alpha {
private int num = 1_000_000;
}
Useless Qualified This
Description: Reports qualified this usages in the same class
public class Alpha {
private class Alpha2 {
final Alpha2 Alpha2 = Alpha2.this;
}
}
public class Alpha {
private class Alpha2 {
final Alpha myAlpha = Alpha.this;
}
}
Avoid Unchecked Exceptions In Signatures
Description: Document the exceptional cases with a @throws Javadoc tag, which allows being more descriptive
public void foo() throws RuntimeException {
}
Clone Method Must Be Public
Description: Prefer using a public method for classes that implement this interface should override Object.clone
public class Alpha implements Cloneable {
@Override
protected Alpha clone() {
}
}
public class Alpha implements Cloneable {
@Override
public Object clone()
}
Clone Method Return Type Must Match Class Name
Description: If a class implements cloneable the return type of the method clone() must be the class name
public class Alpha implements Cloneable {
@Override
protected Object beta() {
}
}
public class Alpha implements Cloneable {
@Override
protected Alpha beta() { // Violation, Object must be Foo
}
}
Detached Test Case
Description: The method appears to be a test case, but is a member of a class that has one or more JUnit test cases
public class MyTest {
@Test
public void someTest() {
}
public void someOtherTest () {
}
}
public class MyTest {
@Test
public void someTest() {
}
}
Do Not Extend Java Lang Throwable
Description: Extend Exception or RuntimeException instead of Throwable
public class Alpha extends Throwable { }
Do Not Terminate VM
Description: Web applications should not call System.exit(), since only the web container or the application server should stop the JVM
public void alpha() {
System.exit(0);
}
public void alpha() {
Runtime.getRuntime().exit(0);
}
Invalid Log Message Format
Description: Check for messages in slf4j and log4j2 loggers with non matching number of arguments and placeholders
LOGGER.error("forget the arg {}");
LOGGER.error("forget the arg %s");
LOGGER.error("too many args {}", "arg1", "arg2");
LOGGER.error("param {}", "arg1", new IllegalStateException("arg"));
Single Method Singleton
Description: The instance created using the overloaded method is not cached and so, for each call and new objects will be created for every invocation
public static Singleton getInstance(Object obj){
Singleton singleton = (Singleton) obj;
return singleton;
}
public static Singleton getInstance( ) {
return singleton;
}
Singleton Class Returning New Instance
Description: getInstance method always creates a new object and hence does not comply to Singleton Design Pattern behaviour
class Singleton {
private static Singleton instance = null;
public static Singleton getInstance() {
synchronized(Singleton.class) {
return new Singleton();
}
}
}
Unsynchronized Static Formatter
Description: Static Formatter objects should be accessed in a synchronized manner
public class Alpha {
private static final SimpleDateFormat sdf = new SimpleDateFormat();
void alpha() {
sdf.format();
}
}
public class Alpha {
private static final SimpleDateFormat sdf = new SimpleDateFormat();
void alpha() {
synchronized (sdf) {
sdf.format();
}
}
}
Avoid Calendar Date Creation
Description: Use new Date(), java.time.LocalDateTime.now() or ZonedDateTime.now()
public class DateAdd {
private Date bad() {
return Calendar.getInstance().getTime(); // now
}
}
public class DateAdd {
private Date good() {
return new Date(); // now
}
}
Avoid File Stream
Description: Avoid instantiating FileInputStream, FileOutputStream, FileReader, or FileWriter
FileInputStream fis = new FileInputStream(fileName);
try(InputStream is = Files.newInputStream(Paths.get(fileName))) {
}
Hard Coded Crypto Key
Description: Do not use hard coded encryption keys. Better store keys outside of source code
public class Alpha {
void bad() {
SecretKeySpec secretKeySpec = new SecretKeySpec("my secret here".getBytes(), "AES");
}
}
public class Alpha {
void good() {
SecretKeySpec secretKeySpec = new SecretKeySpec(Properties.getKey(), "AES");
}
}
Insecure Crypto Iv
Description: Do not use hard coded initialization vector in crypto operations. Better use a randomly generated IV
public class Alpha {
void bad() {
byte[] iv = new byte[] { 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, };
}
}
public class Alpha {
void good() {
SecureRandom random = new SecureRandom();
byte iv[] = new byte[16];
random.nextBytes(bytes);
}
}
