Design

The Design ruleset contains rules that flag suboptimal code implementations. Alternate approaches are suggested.

UseSingleton

Since: PMD 0.3

For classes that only have static methods, consider making them Singletons. Note that this doesn't apply to abstract classes, since their subclasses may well include non-static methods. Also, if you want this class to be a Singleton, remember to add a private constructor to prevent instantiation.

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.design.UseSingletonRule

Example(s):

public class MaybeASingleton {
  public static void foo() {}
  public static void bar() {}
}

    

SimplifyBooleanReturns

Since: PMD 0.9

Avoid unnecessary if-then-else statements when returning a boolean. The result of the conditional test can be returned instead.

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.design.SimplifyBooleanReturnsRule

Example(s):

public boolean isBarEqualTo(int x) {
   
	if (bar == x) {		 // this bit of code...
		return true;
	} else {
		return false;
    }
}

public boolean isBarEqualTo(int x) {

   	return bar == x;	// can be replaced with this
}

    

SimplifyBooleanExpressions

Since: PMD 1.05

Avoid unnecessary comparisons in boolean expressions, they serve no purpose and impacts readability.


//EqualityExpression/PrimaryExpression
 /PrimaryPrefix/Literal/BooleanLiteral

              
Example(s):
  
public class Bar {
  // can be simplified to
  // bar = isFoo();
  private boolean bar = (isFoo() == true);

  public isFoo() { return false;}
}
  
      

SwitchStmtsShouldHaveDefault

Since: PMD 1.0

All switch statements should include a default option to catch any unspecified values.

                  
//SwitchStatement[not(SwitchLabel[@Default='true'])]
                  
              
Example(s):

public void bar() {
    int x = 2;
    switch (x) {
      case 1: int j = 6;
      case 2: int j = 8;
      				// missing default: here
    }
}

    

AvoidDeeplyNestedIfStmts

Since: PMD 1.0

Avoid creating deeply nested if-then statements since they are harder to read and error-prone to maintain.

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.design.AvoidDeeplyNestedIfStmtsRule

Example(s):

public class Foo {
  public void bar(int x, int y, int z) {
    if (x>y) {
      if (y>z) {
        if (z==x) {
         // !! too deep
        }
      }
    }
  }
}

    

AvoidReassigningParameters

Since: PMD 1.0

Reassigning values to incoming parameters is not recommended. Use temporary local variables instead.

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.design.AvoidReassigningParametersRule

Example(s):

public class Foo {
  private void foo(String bar) {
    bar = "something else";
  }
}

    

SwitchDensity

Since: PMD 1.02

A high ratio of statements to labels in a switch statement implies that the switch statement is overloaded. Consider moving the statements into new methods or creating subclasses based on the switch variable.

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.design.SwitchDensityRule

Example(s):
 
public class Foo {
  public void bar(int x) {
    switch (x) {
      case 1: {
        // lots of statements
        break;
      } case 2: {
        // lots of statements
        break;
      }
    }
  }
}
 
      

ConstructorCallsOverridableMethod

Since: PMD 1.04

Calling overridable methods during construction poses a risk of invoking methods on an incompletely constructed object and can be difficult to debug. It may leave the sub-class unable to construct its superclass or forced to replicate the construction process completely within itself, losing the ability to call super(). If the default constructor contains a call to an overridable method, the subclass may be completely uninstantiable. Note that this includes method calls throughout the control flow graph - i.e., if a constructor Foo() calls a private method bar() that calls a public method buz(), this denotes a problem.

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.design.ConstructorCallsOverridableMethodRule

Example(s):
  
public class SeniorClass {
  public SeniorClass(){
      toString(); //may throw NullPointerException if overridden
  }
  public String toString(){
    return "IAmSeniorClass";
  }
}
public class JuniorClass extends SeniorClass {
  private String name;
  public JuniorClass(){
    super(); //Automatic call leads to NullPointerException
    name = "JuniorClass";
  }
  public String toString(){
    return name.toUpperCase();
  }
}
  
      

AccessorClassGeneration

Since: PMD 1.04

Instantiation by way of private constructors from outside of the constructor's class often causes the generation of an accessor. A factory method, or non-privatization of the constructor can eliminate this situation. The generated class file is actually an interface. It gives the accessing class the ability to invoke a new hidden package scope constructor that takes the interface as a supplementary parameter. This turns a private constructor effectively into one with package scope, and is challenging to discern.

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.design.AccessorClassGenerationRule

Example(s):
  
public class Outer {
 void method(){
  Inner ic = new Inner();//Causes generation of accessor class
 }
 public class Inner {
  private Inner(){}
 }
}
  
      

FinalFieldCouldBeStatic

Since: PMD 1.1

If a final field is assigned to a compile-time constant, it could be made static, thus saving overhead in each object at runtime.

                    
//FieldDeclaration
 [@Final='true' and @Static='false']
 [not (../../../../ClassOrInterfaceDeclaration[@Interface='true'])]
   /VariableDeclarator/VariableInitializer/Expression
    /PrimaryExpression/PrimaryPrefix/Literal
                    
                
Example(s):
  
public class Foo {
  public final int BAR = 42; // this could be static and save some space
}
  
      

CloseResource

Since: PMD 1.2.2

Ensure that resources (like Connection, Statement, and ResultSet objects) are always closed after use.

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.design.CloseResourceRule

Example(s):

public class Bar {
  public void foo() {
    Connection c = pool.getConnection();
    try {
      // do stuff
    } catch (SQLException ex) {
     // handle exception
    } finally {
      // oops, should close the connection using 'close'!
      // c.close();
    }
  }
}

    

NonStaticInitializer

Since: PMD 1.5

A non-static initializer block will be called any time a constructor is invoked (just prior to invoking the constructor). While this is a valid language construct, it is rarely used and is confusing.


//Initializer[@Static='false']

                 
Example(s):
   
public class MyClass {
 // this block gets run before any call to a constructor
  {
   System.out.println("I am about to construct myself");
  }
}
   
       

DefaultLabelNotLastInSwitchStmt

Since: PMD 1.5

By convention, the default label should be the last label in a switch statement.


//SwitchStatement
 [not(SwitchLabel[position() = last()][@Default='true'])]
 [SwitchLabel[@Default='true']]

                 
Example(s):
   
public class Foo {
  void bar(int a) {
   switch (a) {
    case 1:  // do something
       break;
    default:  // the default case should be last, by convention
       break;
    case 2:
       break;
   }
  }
}   
       

NonCaseLabelInSwitchStatement

Since: PMD 1.5

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.

 
//SwitchStatement//BlockStatement/Statement/LabeledStatement
 
                 
Example(s):
   
public class Foo {
  void bar(int a) {
   switch (a) {
     case 1:
       // do something
       break;
     mylabel: // this is legal, but confusing!
       break;
     default:
       break;
    }
  }
}
   
       

OptimizableToArrayCall

Since: PMD 1.8

Calls to a collection's toArray() method should specify target arrays sized to match the size of the collection. Initial arrays that are too small are discarded in favour of new ones that have to be created that are the proper size.

                  
//PrimaryExpression
[PrimaryPrefix/Name[ends-with(@Image, 'toArray')]]
[
PrimarySuffix/Arguments/ArgumentList/Expression
 /PrimaryExpression/PrimaryPrefix/AllocationExpression
 /ArrayDimsAndInits/Expression/PrimaryExpression/PrimaryPrefix/Literal[@Image='0']
]

                  
              
Example(s):
  
List foos = getFoos();
  
    // inefficient, the array will be discarded
Foo[] fooArray = foos.toArray(new Foo[0]);
    
    // much better; this one sizes the destination array, 
    // avoiding of a new one via reflection
Foo[] fooArray = foos.toArray(new Foo[foos.size()]);
  
      

BadComparison

Since: PMD 1.8

Avoid equality comparisons with Double.NaN. Due to the implicit lack of representation precision when comparing floating point numbers these are likely to cause logic errors.

                  
//EqualityExpression[@Image='==']
 /PrimaryExpression/PrimaryPrefix
 /Name[@Image='Double.NaN' or @Image='Float.NaN']
                  
              
Example(s):
  
boolean x = (y == Double.NaN);
  
      

EqualsNull

Since: PMD 1.9

Tests for null should not use the equals() method. The '==' operator should be used instead.

    
//PrimaryExpression
 [
PrimaryPrefix/Name[ends-with(@Image, 'equals')]
or
PrimarySuffix[ends-with(@Image, 'equals')]
]
[PrimarySuffix/Arguments/ArgumentList[count(Expression)=1]
  /Expression/PrimaryExpression/PrimaryPrefix
   /Literal/NullLiteral]
    
                
Example(s):
       
String x = "foo";

if (x.equals(null)) { // bad form
   	doSomething();
	}
	
if (x == null) { 	// preferred
   	doSomething();
	}
    
        

ConfusingTernary

Since: PMD 1.9

Avoid negation within an "if" expression with an "else" clause. For example, rephrase: if (x != y) diff(); else same(); as: if (x == y) same(); else diff(); Most "if (x != y)" cases without an "else" are often return cases, so consistent use of this rule makes the code easier to read. Also, this resolves trivial ordering problems, such as "does the error case go first?" or "does the common case go first?".

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.design.ConfusingTernaryRule

Example(s):
          
boolean bar(int x, int y) {
  return (x != y) ? diff : same;
 }
          
        

InstantiationToGetClass

Since: PMD 2.0

Avoid instantiating an object just to call getClass() on it; use the .class public member instead.

                
//PrimarySuffix
 [@Image='getClass']
 [parent::PrimaryExpression
  [PrimaryPrefix/AllocationExpression]
  [count(PrimarySuffix) = 2]
 ]
     
            
Example(s):
    
  // replace this
Class c = new String().getClass();

  // with this:
Class c = String.class;

    
        

IdempotentOperations

Since: PMD 2.0

Avoid idempotent operations - they have no effect.

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.design.IdempotentOperationsRule

Example(s):
      
public class Foo {
 public void bar() {
  int x = 2;
  x = x;
 }
}
      
      

SimpleDateFormatNeedsLocale

Since: PMD 2.0

Be sure to specify a Locale when creating SimpleDateFormat instances to ensure that locale-appropriate formatting is used.


//AllocationExpression
 [ClassOrInterfaceType[@Image='SimpleDateFormat']]
 [Arguments[@ArgumentCount=1]]

                    
Example(s):
        
public class Foo {
  // Should specify Locale.US (or whatever)
  private SimpleDateFormat sdf = new SimpleDateFormat("pattern");
}
        
        

ImmutableField

Since: PMD 2.0

Identifies private fields whose values never change once they are initialized either in the declaration of the field or by a constructor. This helps in converting existing classes to becoming immutable ones.

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.design.ImmutableFieldRule

Example(s):
  
public class Foo {
  private int x; // could be final
  public Foo() {
      x = 7;
  }
  public void foo() {
     int a = x + 2;
  }
}
  
      

UseLocaleWithCaseConversions

Since: PMD 2.0

When doing String.toLowerCase()/toUpperCase() conversions, use Locales to avoids problems with languages that have unusual conventions, i.e. Turkish.

                
//PrimaryExpression
[PrimaryPrefix/Name
 [ends-with(@Image, 'toLowerCase') or ends-with(@Image,
'toUpperCase')]
 ]
[PrimarySuffix[position() = 1]/Arguments[@ArgumentCount=0]]
     
            
Example(s):
    
class Foo {
 // BAD
 if (x.toLowerCase().equals("list"))...
 /*
 This will not match "LIST" when in Turkish locale
 The above could be
 if (x.toLowerCase(Locale.US).equals("list")) ...
 or simply
 if (x.equalsIgnoreCase("list")) ...
 */
 // GOOD
 String z = a.toLowerCase(Locale.EN);
}
    
        

AvoidProtectedFieldInFinalClass

Since: PMD 2.1

Do not use protected fields in final classes since they cannot be subclassed. Clarify your intent by using private or package access modifiers instead.


//ClassOrInterfaceDeclaration[@Final='true']
/ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration
/FieldDeclaration[@Protected='true']
 
                 
Example(s):

public final class Bar {
  private int x;
  protected int y;  // bar cannot be subclassed, so is y really private or package visible?
  Bar() {}
}
 
         

AssignmentToNonFinalStatic

Since: PMD 2.2

Identifies a possible unsafe usage of a static field.

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.design.AssignmentToNonFinalStaticRule

Example(s):
   
public class StaticField {
   static int x;
   public FinalFields(int y) {
    x = y; // unsafe
   }
}
   
       

MissingStaticMethodInNonInstantiatableClass

Since: PMD 3.0

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

    
//ClassOrInterfaceDeclaration[@Nested='false'][
( count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration)>0
  and count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration) = count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration[@Private='true']) )
and
count(.//MethodDeclaration[@Static='true'])=0
and
count(.//FieldDeclaration[@Private='false'][@Static='true'])=0
]
    
              
Example(s):

// This class is unusable, since it cannot be
// instantiated (private constructor),
// and no static method can be called.

public class Foo {
  private Foo() {}
  void foo() {}
}


      

AvoidSynchronizedAtMethodLevel

Since: PMD 3.0

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.

    
//MethodDeclaration[@Synchronized='true']
    
              
Example(s):

public class Foo {
  // Try to avoid this
  synchronized void foo() {
  }
  // Prefer this:
  void bar() {
    synchronized(this) {
   }
  }
}

      

MissingBreakInSwitch

Since: PMD 3.0

Switch statements without break or return statements for each case option may indicate problematic behaviour.

    
//SwitchStatement
[count(.//BreakStatement)=0]
[count(SwitchLabel) > 0]
[count(BlockStatement/Statement/ReturnStatement)
 + count(BlockStatement/Statement/ThrowStatement)
     < count (SwitchLabel)]
    
              
Example(s):

public void bar(int status) {
    switch(status) {
      case CANCELLED:
        doCancelled();
        // break; hm, should this be commented out?
      case NEW:
        doNew();
      case REMOVED:
        doRemoved();
    }
}

      

UseNotifyAllInsteadOfNotify

Since: PMD 3.0

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; thus its usually safer to call notifyAll() instead.

    
//StatementExpression/PrimaryExpression
[count(PrimarySuffix/Arguments/ArgumentList) = 0]
[
PrimaryPrefix[./Name[@Image='notify' or ends-with(@Image,'.notify')]
or @Image='notify'
or (./AllocationExpression and ../PrimarySuffix[@Image='notify'])
]
]
    
              
Example(s):

  void bar() {
    x.notify();
    // If many threads are monitoring x, only one (and you won't know which) will be notified.
    // use instead:
    x.notifyAll();
  }

      

AvoidInstanceofChecksInCatchClause

Since: PMD 3.0

Each caught exception type should be handled in its own catch clause.

    
//CatchStatement/FormalParameter
 /following-sibling::Block//InstanceOfExpression/PrimaryExpression/PrimaryPrefix
  /Name[
   @Image = ./ancestor::Block/preceding-sibling::FormalParameter
    /VariableDeclaratorId/@Image
  ]
    
              
Example(s):

try { // Avoid this
 // do something
} catch (Exception ee) {
 if (ee instanceof IOException) {
  cleanup();
 }
}
try {  // Prefer this:
 // do something
} catch (IOException ee) {
 cleanup();
}

      

AbstractClassWithoutAbstractMethod

Since: PMD 3.0

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. If the class is intended to be used as a base class only (not to be instantiated directly) a protected constructor can be provided prevent direct instantiation.

//ClassOrInterfaceDeclaration
 [@Abstract='true'
  and count( .//MethodDeclaration[@Abstract='true'] )=0 ]
  [count(ImplementsList)=0]
  [count(.//ExtendsList)=0]
              
              
Example(s):

public abstract class Foo {
  void int method1() { ... }
  void int method2() { ... }
  // consider using abstract methods or removing
  // the abstract modifier and adding protected constructors
}

      

SimplifyConditional

Since: PMD 3.1

No need to check for null before an instanceof; the instanceof keyword returns false when given a null argument.

                      
//Expression
 [ConditionalOrExpression
 [EqualityExpression[@Image='==']
  //NullLiteral
  and
  UnaryExpressionNotPlusMinus
   [@Image='!']//InstanceOfExpression[PrimaryExpression
     //Name/@Image = ancestor::ConditionalOrExpression/EqualityExpression
      /PrimaryExpression/PrimaryPrefix/Name/@Image]]
or
ConditionalAndExpression
 [EqualityExpression[@Image='!=']//NullLiteral
 and
InstanceOfExpression
 [PrimaryExpression[count(PrimarySuffix[@ArrayDereference='true'])=0]
  //Name[not(contains(@Image,'.'))]/@Image = ancestor::ConditionalAndExpression
   /EqualityExpression/PrimaryExpression/PrimaryPrefix/Name/@Image]]]
 
                  
Example(s):
      
class Foo {
  void bar(Object x) {
    if (x != null && x instanceof Bar) {
      // just drop the "x != null" check
    }
  }
}      
           

CompareObjectsWithEquals

Since: PMD 3.2

Use equals() to compare object references; avoid comparing them with ==.

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.design.CompareObjectsWithEqualsRule

Example(s):

class Foo {
  boolean bar(String a, String b) {
    return a == b;
  }
}


  

PositionLiteralsFirstInComparisons

Since: PMD 3.3

Position literals first in comparisons, if the second argument is null then NullPointerExceptions can be avoided, they will just return false.

              
//PrimaryExpression[
        PrimaryPrefix[Name
                [
	(ends-with(@Image, '.equals'))
                ]
        ]
        [
                   (../PrimarySuffix/Arguments/ArgumentList/Expression/PrimaryExpression/PrimaryPrefix/Literal)
	and
	( count(../PrimarySuffix/Arguments/ArgumentList/Expression) = 1 )
        ]
]
[not(ancestor::Expression/ConditionalAndExpression//EqualityExpression[@Image='!=']//NullLiteral)]
[not(ancestor::Expression/ConditionalOrExpression//EqualityExpression[@Image='==']//NullLiteral)]

          
          
Example(s):

class Foo {
  boolean bar(String x) {
    return x.equals("2"); // should be "2".equals(x)
  }
}


  

UnnecessaryLocalBeforeReturn

Since: PMD 3.3

Avoid the creation of unnecessary local variables

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.design.UnnecessaryLocalBeforeReturnRule

Example(s):
  
public class Foo {
   public int foo() {
     int x = doSomething();
     return x;  // instead, just 'return doSomething();'
   }
}
  
      

NonThreadSafeSingleton

Since: PMD 3.4

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. Other possible fixes are to synchronize the entire method or to use an initialize-on-demand holder class (do not use the double-check idiom). See Effective Java, item 48.

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.design.NonThreadSafeSingletonRule

Example(s):
private static Foo foo = null;

//multiple simultaneous callers may see partially initialized objects
public static Foo getFoo() {
    if (foo==null)
        foo = new Foo();
    return foo;
}
        

UncommentedEmptyMethod

Since: PMD 3.4

Uncommented Empty Method finds instances where a method does not contain statements, but there is no comment. By explicitly commenting empty methods it is easier to distinguish between intentional (commented) and unintentional empty methods.

    
//MethodDeclaration/Block[count(BlockStatement) = 0 and @containsComment = 'false']
 
             
Example(s):
  
public void doSomething() {
}
 
      

UncommentedEmptyConstructor

Since: PMD 3.4

Uncommented Empty Constructor finds instances where a constructor does not contain statements, but there is no comment. By explicitly commenting empty constructors it is easier to distinguish between intentional (commented) and unintentional empty constructors.

    
//ConstructorDeclaration[@Private='false'][count(BlockStatement) = 0 and ($ignoreExplicitConstructorInvocation = 'true' or not(ExplicitConstructorInvocation)) and @containsComment = 'false']
 
             
Example(s):
  
public Foo() {
  super();
}
 
      

This rule has the following properties:

Name Default Value Description
ignoreExplicitConstructorInvocation false Ignore explicit constructor invocation when deciding whether constructor is empty or not

AvoidConstantsInterface

Since: PMD 3.5

An interface should be used only to characterize the external behaviour of an implementing class: using an interface as a container of constants is a poor usage pattern and not recommended.

    
//ClassOrInterfaceDeclaration[@Interface="true"]
    [
     count(.//MethodDeclaration)=0
     and
     count(.//FieldDeclaration)>0
    ]
    
        
Example(s):
    
public interface ConstantsInterface {
   public static final int CONSTANT1=0;
   public static final String CONSTANT2="1";
}
    
      

UnsynchronizedStaticDateFormatter

Since: PMD 3.6

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 either on method or block level.

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.design.UnsynchronizedStaticDateFormatterRule

Example(s):
    
public class Foo {
    private static final SimpleDateFormat sdf = new SimpleDateFormat();
    void bar() {
        sdf.format(); // poor, no thread-safety
    }
    synchronized void foo() {
        sdf.format(); // preferred
    }
}
    
      

PreserveStackTrace

Since: PMD 3.7

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.

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.design.PreserveStackTraceRule

Example(s):
    
public class Foo {
    void good() {
        try{
            Integer.parseInt("a");
        } catch(Exception e){
            throw new Exception(e);
        }
    }
    void bad() {
        try{
            Integer.parseInt("a");
        } catch(Exception e){
            throw new Exception(e.getMessage());
        }
    }
}
    
      

UseCollectionIsEmpty

Since: PMD 3.9

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.

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.design.UseCollectionIsEmptyRule

Example(s):
    
public class Foo {
	void good() {
       	List foo = getList();
		if (foo.isEmpty()) {
			// blah
		}
   	}

    void bad() {
   	    List foo = getList();
			if (foo.size() == 0) {
				// blah
			}
    	}
}
    
      

ClassWithOnlyPrivateConstructorsShouldBeFinal

Since: PMD 4.1

A class with only private constructors should be final, unless the private constructor is invoked by a inner class.

TypeDeclaration[count(../TypeDeclaration) = 1]/ClassOrInterfaceDeclaration
[@Final = 'false']
[count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration[@Private = 'true']) >= 1 ]
[count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration[(@Public = 'true') or (@Protected = 'true') or (@PackagePrivate = 'true')]) = 0 ]
[not(.//ClassOrInterfaceDeclaration)]
             
Example(s):
public class Foo {  //Should be final
    private Foo() { }
}
     

EmptyMethodInAbstractClassShouldBeAbstract

Since: PMD 4.1

Empty methods in an abstract class should be tagged as abstract. This helps to remove their inapproprate usage by developers who should be implementing their own versions in the concrete subclasses.

                
                    //ClassOrInterfaceDeclaration[@Abstract = 'true']
                        /ClassOrInterfaceBody
                        /ClassOrInterfaceBodyDeclaration
                        /MethodDeclaration[@Abstract = 'false' and @Native = 'false']
                        [
                            ( boolean(./Block[count(./BlockStatement) =  1]/BlockStatement/Statement/ReturnStatement/Expression/PrimaryExpression/PrimaryPrefix/Literal/NullLiteral) = 'true' )
                            or
                            ( boolean(./Block[count(./BlockStatement) =  1]/BlockStatement/Statement/ReturnStatement/Expression/PrimaryExpression/PrimaryPrefix/Literal[@Image = '0']) = 'true' )
                    		or
							( boolean(./Block[count(./BlockStatement) =  1]/BlockStatement/Statement/ReturnStatement/Expression/PrimaryExpression/PrimaryPrefix/Literal[string-length(@Image) = 2]) = 'true' )
							or
							(
								(
									(boolean(./Block/BlockStatement/Statement/ReturnStatement/Expression/PrimaryExpression/PrimaryPrefix/Literal[@Image = '']) = 'true' )
								)
								and
								( count (./Block/*) = 1 )
							)
                            or
                            ( count (./Block/*) = 0 )
                        ]
                
             
Example(s):
        	
public abstract class ShouldBeAbstract {
    public Object couldBeAbstract()  {
					// Should be abstract method ?
			return null;
	}

	public void couldBeAbstract()  {
    }
}
	     	
    	

SingularField

Since: PMD 3.1

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.

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.design.SingularFieldRule

Example(s):
public class Foo {
    private int x;  // no reason to exist at the Foo instance level
    public void foo(int y) {
     x = y + 5;
     return x;
    }
}
   

ReturnEmptyArrayRatherThanNull

Since: PMD 4.2

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.

                    
                        //MethodDeclaration
                        [
                        (./ResultType/Type[@Array='true'])
                        and
                        (./Block/BlockStatement/Statement/ReturnStatement/Expression/PrimaryExpression/PrimaryPrefix/Literal/NullLiteral)
                        ]
                    
                
Example(s):
public class Example {
    // Not a good idea...
    public int[] badBehavior() {
                   // ...
        return null;
    }

    // Good behavior
    public String[] bonnePratique() {
      //...
     return new String[0];
    }
}
            

AbstractClassWithoutAnyMethod

Since: PMD 4.2

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. In this case, it is probably better to use a private or protected constructor in order to prevent instantiation than make the class misleadingly abstract.

                    
//ClassOrInterfaceDeclaration[
	(@Abstract = 'true')
	and
	(count(//MethodDeclaration) + count(//ConstructorDeclaration) = 0)
]
                    
                
Example(s):
            
public class abstract Example {
	String field;
	int otherField;
}
            
        

TooFewBranchesForASwitchStatement

Since: PMD 4.2

Switch statements are indended 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. In these cases use the if-then statement to increase code readability.

                    
//SwitchStatement[
	(count(.//SwitchLabel) < $minimumNumberCaseForASwitch)
]
                    
                
Example(s):
            
// With a minimumNumberCaseForASwitch of 3	    
public class Foo {
	public void bar() {
		switch (condition) {
             		case ONE:
	        	        instruction;
				break;
	        	default:
				break; // not enough for a 'switch' stmt, a simple 'if' stmt would have been more appropriate
		}
	}
}
            
        

This rule has the following properties:

Name Default Value Description
minimumNumberCaseForASwitch 3 Minimum number of branches for a switch

LogicInversion

Since: PMD 5.0

Use opposite operator instead of negating the whole expression with a logic complement operator.

          
//UnaryExpressionNotPlusMinus[@Image='!']/PrimaryExpression/PrimaryPrefix/Expression[EqualityExpression or RelationalExpression]
          
          
Example(s):
    
public boolean bar(int a, int b) {

	if (!(a == b)) // use !=
         return false;

	if (!(a < b)) // use >=
         return false;

	return true;
}
    
    

UseVarargs

Since: PMD 5.0

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.

//FormalParameters/FormalParameter[position()=last() and @Array='true' and @Varargs='false']
					
Example(s):
public class Foo {
   public void foo(String s, Object[] args) {
      // Do something here...
   }

   public void bar(String s, Object... args) {
      // Ahh, varargs tastes much better...
   }
}
        

FieldDeclarationsShouldBeAtStartOfClass

Since: PMD 5.0

Fields should be declared at the top of the class, before any method declarations, constructors, initializers or inner classes.

          
//ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/FieldDeclaration
 [count(../preceding-sibling::ClassOrInterfaceBodyDeclaration/child::*[1][name() != 'FieldDeclaration' and name() != 'Annotation']) > 0]
          
        
Example(s):
      
public class HelloWorldBean {

  // Field declared before methods / inner classes - OK
  private String _thing;

  public String getMessage() {
    return "Hello World!";
  }

  // Field declared after methods / inner classes - avoid this
  private String _fieldInWrongLocation;
}
      
    

GodClass

Since: PMD 5.0

The God Class rule detects the God Class design flaw using metrics. God classes do too many things, are very big and overly complex. They should be split apart to be more object-oriented. The rule uses the detection strategy described in "Object-Oriented Metrics in Practice". The violations are reported against the entire class. See also the references: Michele Lanza and Radu Marinescu. Object-Oriented Metrics in Practice: Using Software Metrics to Characterize, Evaluate, and Improve the Design of Object-Oriented Systems. Springer, Berlin, 1 edition, October 2006. Page 80.

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.design.GodClassRule