PMD Coding practices - Design in Java

Design practices (Java) - provided by PMD

UseSingleton

For classes that only have static methods, consider making them Singletons.

Note that this doesn't apply to abstract classes, since their subclasses maywell include non-static methods.

Also, if you want this class to be a Singleton,remember to add a private constructor to prevent instantiation.

SimplifyBooleanReturns

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

SimplifyBooleanExpressions

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

SwitchStmtsShouldHaveDefault

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

AvoidDeeplyNestedIfStmts

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

AvoidReassigningParameters

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

SwitchDensity

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.

ConstructorCallsOverridableMethod

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.

AccessorClassGeneration

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.

FinalFieldCouldBeStatic

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

CloseResource

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

NonStaticInitializer

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.

DefaultLabelNotLastInSwitchStmt

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

NonCaseLabelInSwitchStatement

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.

OptimizableToArrayCall

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 createdthat are the proper size.

BadComparison

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

EqualsNull

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

ConfusingTernary

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, suchas "does the error case go first?" or "does the common case go first?".

InstantiationToGetClass

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

IdempotentOperations

Avoid idempotent operations - they have no effect.

SimpleDateFormatNeedsLocale

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

ImmutableField

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.

UseLocaleWithCaseConversions

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

AvoidProtectedFieldInFinalClass

Do not use protected fields in final classes since they cannot be subclassed.

Clarify your intent by using private or package access modifiers instead.

AssignmentToNonFinalStatic

Identifies a possible unsafe usage of a static field.

MissingStaticMethodInNonInstantiatableClass

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

AvoidSynchronizedAtMethodLevel

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.

MissingBreakInSwitch

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

Empty cases are ignored as these indicate an intentional fall-through.

UseNotifyAllInsteadOfNotify

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.

AvoidInstanceofChecksInCatchClause

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

AbstractClassWithoutAbstractMethod

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.

SimplifyConditional

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

CompareObjectsWithEquals

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

PositionLiteralsFirstInComparisons

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

UnnecessaryLocalBeforeReturn

Avoid the creation of unnecessary local variables

NonThreadSafeSingleton

Non-thread safe singletons can result in bad state changes.

Eliminate static singletons if possible by instantiating the object directly. Staticsingletons are usually not needed as only a single instance exists anyway.

Other possible fixes are to synchronize the entire method or to use aninitialize-on-demand holder class (do not use the double-check idiom)

UncommentedEmptyMethod

Uncommented Empty Method finds instances where a method does not containstatements, but there is no comment.

By explicitly commenting empty methodsit is easier to distinguish between intentional (commented) and unintentionalempty methods.

UncommentedEmptyConstructor

Uncommented Empty Constructor finds instances where a constructor does notcontain statements, but there is no comment.

By explicitly commenting emptyconstructors it is easier to distinguish between intentional (commented)and unintentional empty constructors.

AvoidConstantsInterface

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

UnsynchronizedStaticDateFormatter

SimpleDateFormat instances are not synchronized. Sun recommends using separate format instancesfor each thread.

If multiple threads must access a static formatter, the formatter must be synchronized either on method or block level.

PreserveStackTrace

Throwing a new exception from a catch block without passing the original exception into thenew exception will cause the original stack trace to be lost making it difficult to debug effectively.

UseCollectionIsEmpty

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.

ClassWithOnlyPrivateConstructorsShouldBeFinal

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

EmptyMethodInAbstractClassShouldBeAbstract

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.

SingularField

Fields whose scopes are limited to just single methods do not rely on the containingobject to provide them to other methods.

They may be better implemented as local variableswithin those methods.

ReturnEmptyArrayRatherThanNull

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 inadvertentNullPointerExceptions.

AbstractClassWithoutAnyMethod

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.

TooFewBranchesForASwitchStatement

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 theif-then statement to increase code readability.

LogicInversion

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

UseVarargs

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.

FieldDeclarationsShouldBeAtStartOfClass

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

GodClass

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.