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