J2SE Best Practices

Rules

Name	Severity	Description
RuleAwtPeer	Severe	java.awt.peer interfaces were designed to handle platform-specific graphics issues in Java. Since Java is meant to be platform-independent, using java.awt.peer directly is forbidden.
RuleCloneableSuperClone	Severe	Calling super.clone() from the clone() is essential in order to ensure that a complete copy of the object is created. Failure to do so is likely to result in a NullPointerException thrown later or an unexpected object state in the course of program execution.
RuleComparisonEqualsHashCode	Severe	java.lang.Object.equals() and java.lang.Object.hashCode() are designed to work together for fast hash lookups. The value returned from the hashCode() is used to calculate the index of the bucket in the hash table array. The equals() is then used to find the actual matching object among all the objects that sit in the same bucket.
RuleComparisonClassNameComparison	Severe	Using java.lang.Class.getName() bypasses the standard way of comparing classes and may lead to problems, including security violations. A standard way of comparing class objects is to use java.lang.Class.equals(). This method bases the class equality on all aspects of the class, for example, the ClassLoader that loaded the class.
RuleNullEmptyArray	Severe	Returning null instead of empty array is error-prone in majority of cases. By Java convention, the user of the interface that returns array does not have to check for null, but should assume the zero length array. Violating this convention is likely to result in null pointer exceptions.
RuleSerializationReadWriteObject	Severe	Declaring readObject() and writeObject() private is essential to follow Java serialization protocol. Failing to do so may lead to security problems in the application.
RuleSerializationResolveObject	Severe	Declaring resolveObject() protected is essential in order to follow Java serialization protocol. Failing to do so may lead to security problems in the application.
RuleSerializationSerializableField	Severe	Every field of a serializable class must be either serializable or transient. Declaring non-transient fields of non-serializable type inside of a serializable class will result in an exception thrown during the serialization.
nonsynchronizednotify	Severe	A thread that is currently locking sections of the code that use a common lock should invoke java.lang.Object.notify() to tell the blocking thread to resume processing. Calling notify() from non-synchronized blocks will result in IllegalMonitorStateException, because no lock has been obtained.
nonsynchronizednotifyall	Severe	A thread that is currently locking sections of the code that use common lock should invoke java.lang.Object.notifyAll() to tell the blocking threads to resume processing. Calling notify() from non-synchronized blocks will result in IllegalMonitorStateException, because no lock has been obtained.
nonsynchronizedwait	Severe	A thread that needs to wait on a common lock should invoke java.lang.Object.wait() to tell the JVM to suspend its processing until it receives notification from another thread. Calling wait() from non-synchronized blocks will result in IllegalMonitorStateException, because no lock has been obtained.
destroy	Severe	java.lang.Thread.destroy() method destroys the thread without releasing the monitors or doing a cleanup. Calling this method is likely to lead to synchronization and resource management issues.
resume	Severe	The following is the extract from Java doc (copyright by Sun Microsystems): This method exists solely for use with suspend(), which has been deprecated because it is deadlock-prone.
stop	Severe	The following is the extract from Java doc (copyright by Sun Microsystems): This method is inherently unsafe. Stopping a thread with Thread.stop causes it to unlock all of the monitors that it has locked (as a natural consequence of the unchecked ThreadDeath exception propagating up the stack). If any of the objects previously protected by these monitors were in an inconsistent state, the damaged objects become visible to other threads, potentially resulting in arbitrary behavior.
suspend	Severe	The following is the extract from Java doc (copyright by Sun Microsystems): This method has been deprecated, as it is inherently deadlock-prone. If the target thread holds a lock on the monitor protecting a critical system resource when it is suspended, no thread can access this resource until the target thread is resumed. If the thread that would resume the target thread attempts to lock this monitor prior to calling resume, deadlock results. Such deadlocks typically manifest themselves as "frozen" processes.
overridesynchronizable	Severe	Synchronized methods use this as a lock to prevent other methods and blocks with the same lock from parallel execution. Overriding the synchronized method with a non-synchronized one introduces the potential for problems and data inconsistency in the base class.
RuleCastingPrimitives	Warning	Casting primitives to a lower precision may result in the information loss. Such casting indicates an incorrect choice of the data type or a calculation that ignores properties of the data that it operates on.
RuleCloneableOverrideClone	Warning	Implementing Cloneable without overriding clone() is confusing and unclear. By implementing Cloneable, the class signals that the default, shallow copy mechanism, is not sufficient, and that the class needs to perform additional operations in order to create a valid copy.
RuleCloneableImplementCloneable	Warning	Overriding clone() without implementing Cloneable is confusing and unclear. By implementing Cloneable, the class signals that the default, shallow copy mechanism, is not sufficient, and that the class needs to perform additional operations in order to create a valid copy.
RuleComparisonInstanceOfEquals	Warning	Using instanceof in the equals() of an object is critical in order to ensure that equals does not throw a ClassCastException and returns the correct value.
RuleComparisonReferenceEquality	Warning	Operators == and != compare references of objects. In most cases, the two objects should be considered the same if they have the same state. It is better to use java.lang.Object.equals() for comparison, since this method allows for the implementer to make the object state comparison.
RuleExceptionsInstanceofCatch	Warning	Since Java supports multiple catch clauses in the try block, there is never a need to check for the instance of the exception in the catch clause. Catch clauses should be short, clear statements that deal directly with the specific exception that was thrown. Adding instanceof exception checks is likely to lead to non-standard, difficult to maintain code.
RuleLoopAssignLoopVariable	Warning	Assigning the for loop control variable in the body of the loop is likely to lead to an error. For loops already provide the default increment mechanism that should be followed. When the default increment mechanism does not apply, the for loop should be changed to a while or a do loop.
RuleNullEnumeration	Warning	Returning null instead of empty Enumeration is error-prone in majority of cases. By Java convention, the client of the interface that returns Enumeration does not have to check for null, but should assume the empty enumeration that returns false in hasMoreElements(). Violating this convention is likely to result in null pointer exceptions.
RuleNullIterator	Warning	Returning null instead of empty Iterator is error-prone in majority of cases. By Java convention, the client of the interface that returns Iterator does not have to check for null, but should assume the empty Iterator that returns false in hasNext(). Violating this convention is likely to result in null pointer exceptions.
RulePortabilitySystemGetenv	Warning	The following is the extract from Java doc (copyright by Sun Microsystems): The preferred way to extract system-dependent information is the system properties of the java.lang.System.getProperty methods and the corresponding getTypeName methods of the Boolean, Integer, and Long primitive.
RulePortabilityLineSeparators	Warning	Hard coding line separators as '\n' or '\r' makes the code non portable. In Java there is a system property called 'line.separator' specifically designed for portability.
RulePortabilityFileSeparator	Warning	Hard coding a file separator as '/' or '\' makes the code non portable. The java.io.File class contains a special field called 'separator' specifically designed for portability.
RuleSwitchBreak	Warning	If a break is missing in a case branch of a switch statement, the code continues by executing the next case branch. While there are situations when this is the intent, they are quite rare. More often, the intent is not to execute the following case but to break out of the current case branch. Putting the break at the end of each case branch makes code safer and more clear.
RuleCastingObjectDowncast	Recommendation	Returning java.lang.Object is unclear and error prone. The problem can manifest itself as the runtime ClassCastException. In order to avoid this exception, the users of the interface need to read (and often re-read) the documentation and maybe even look into the details of the implementation.
RuleComparisonPlaceConstants	Recommendation	Placing constants on the left side of equals() eliminates the need for extra checking and reduces incidences of null pointer exceptions.
RuleConditionalTernary	Recommendation	Using an if/else statement instead of a ternary operator makes code longer than necessary. A ternary operator makes the code more compact, while preserving the clarity of the if/else statement.
RuleConditionalNegationIfCondition	Recommendation	Negation in if/else conditions makes the code more confusing. Since if/else branches have statements for cases when the condition is either true or false, there is no need to use negation.
RuleConstructorsAbstractMethods	Recommendation	Calling an abstract method in the object constructor of an abstract class is problematic because its subclass may not have been properly initialized. Such calls may result in NullPointerException or an incorrect object state.
RuleConstructorsOverridableMethods	Recommendation	Calling an overridable method in an object constructor is problematic because the subclass may not have been properly initialized. Such calls may result in NullPointerException or an incorrect object state.
RuleDeclarationCArrayStyle	Recommendation	The C-style array declaration is uncommon and archaic in Java. Avoid it because most Java programmers are not familiar with it.
RuleDeclarationMultipleDeclaration	Recommendation	Declaring several variables in the same statement makes the code unclear. Such declaration leads to cumbersome initialization and may increase the time needed to maintain the code.
RuleDeclarationMixedDeclaration	Recommendation	Declaring basic and array variables in the same statement makes the code harder to comprehend. Such declaration leads to cumbersome initialization and may increase the time needed to maintain the code.
RuleDeclarationDeclareConstants	Recommendation	Hard coding string literals is problematic for readability and globalization. To improve the performance of the code and to ensure that it is easy to read and make globalization compliant, avoid declaring explicit string literals, especially duplicate ones.
RuleDeclarationInterfaceConstants	Recommendation	Interfaces in Java provide a mechanism for behavioral inheritance without implementation inheritance. While it is possible to declare constants in an interface and then extend this interface, this is not recommended. Constants should be declared in an interface or a class to which they logically belong. All other classes and interfaces should then refer to the constant declarations.
RuleExceptionsSpecificExceptions	Recommendation	For clarity it is better to maintain the complete list of thrown exceptions in the throws clause.
RuleExceptionsCatchGeneric	Recommendation	Creating broad catch clauses that handle generic exceptions such as java.lang.Exception or java.lang.Throwable exceptions may make it difficult to correct the problem. Since exceptions in Java are designed as regular classes, it is better to create and throw a specific type of an exception and to have a dedicated catch clause for each exception.
RuleExceptionsExtendError	Recommendation	java.lang.Error is a runtime exception, primarily meant to be extended and used for signaling runtime problems encountered by the Java environment. The runtime exceptions are unchecked, that is the compiler will not require the user of the interface to handle it. Generally, it is preferable to create the checked exception instead of the unchecked ones.
RuleExceptionsExtendRuntimeException	Recommendation	java.lang.RuntimeException is a runtime exception, primarily meant to be extended and used for signaling runtime problems encountered by the Java environment. The runtime exceptions are unchecked, that is the compiler will not require the user of the interface to handle it. Generally, it is preferable to create the checked exception instead of the unchecked ones.
RuleExceptionsExtendThrowable	Recommendation	java.lang.Throwable is a common base class for both checked and unchecked exceptions in Java. This class is not meant to be extended directly. It is preferable to create the checked exceptionthat descends from java.lang.Exception.
RuleExceptionsThrowError	Recommendation	java.lang.Error is a runtime exception, primarily meant to be extended and used for signaling runtime problems encountered by the Java environment. The runtime exceptions are unchecked, that is the compiler will not require the user of the interface to handle it. Generally, it is preferable to throw checked exceptions instead of the unchecked ones.
RuleExceptionsThrowException	Recommendation	java.lang.Exception is a generic checked exception in Java. Since exceptions in Java are designed as regular classes, it is better to create and throw specific sub classes of this generic class and to have a dedicated catch clause for each exception.
RuleExceptionsThrowThrowable	Recommendation	java.lang.Throwable is a common base class for both checked and unchecked exceptions in Java. This class is not meant to be used directly in the throws clause. It is preferable to create and use specific checked exceptions thfrom java.lang.Exception.
RuleInitializationInitializeStaticFields	Recommendation	Uninitialzed static fields may lead to problems such as NullPointerException. Also, when a field is not explicitely initialized, someone reading the code may not know the default value and may have difficulty understanding the code.
RuleInitializationChainingAssignments	Recommendation	Chaining assignment operators introduces the potential for errors and makes the code unclear. The code is more robust and self-explanatory if the assignments are separated.
RuleInitializationAssigningMethodParameter	Recommendation	Changing the value of a method parameter alters its original intent and makes the code unclear.
RuleLoopNoConditionForLoop	Recommendation	A for loop without a condition may lead to an infinite loop. Such for loops are also more difficult to understand and maintain.
RuleLoopNoInitIncrForLoop	Recommendation	A for loop without an initializer and an incrementor is misleading and error-prone. Besides hiding the intend, the loop may lead to 'infinite loop' problems.
RuleLoopContinueStatement	Recommendation	Continue statements add additional, unnecessary complexity to the body of a loop. Continue statements typically occur in long loops with non-trivial logic. Such loops need to be refactored into smaller, more clear calculations. After performing the refactoring, it will be possible to replace the continue statement with an if statement without complicating the code.
RuleReflectionGetDeclaredField	Recommendation	Java reflection facilities are designed for generic processing of class content. For example, the reflection is used by Java Beans framework to enable generic inspection of properties of Java Beans. The reflection is not meant to circumvent the standard java field or method access. Thus, calling getDeclaredField() with the hard coded name of the field is not recommended.
RuleReflectionGetDeclaredMethod	Recommendation	Java reflection facilities are designed for generic processing of class content. For example, the reflection is used by Java Beans framework to enable generic inspection of properties of Java Beans. The reflection is not meant to circumvent the standard java field or method access. Thus, calling getDeclaredMethod() with the hard coded name of the method is not desirable.
RuleReflectionGetField	Recommendation	Java reflection facilities are designed for generic processing of class content. For example, the reflection is used by Java Beans framework to enable generic inspection of properties of Java Beans. The reflection is not meant to circumvent the standard java field or method access. Thus, calling getField() with the hard coded name of the field is not desirable.
RuleReflectionGetMethod	Recommendation	Java reflection facilities are designed for generic processing of class content. For example, the reflection is used by Java Beans framework to enable generic inspection of properties of Java Beans. The reflection is not meant to circumvent the standard java field or method access. Thus, calling getMethod() with the hard coded name of the method is not desirable.
RuleSerializationSerialVersionUID	Recommendation	The serialVersionUID field is used by Java serialization to ensure correct deserialization of old objects when the underlying class is altered. Failing to declare the serialVersionUID inside of a serializable class will result in the IOException when attempting to load old instances of this class.
RuleSerializationTransientField	Recommendation	The transient keyword is used in Java to denote the non-serializable fields inside of serializable classes. Using this keyword inside of non-serializable classes makes the code unclear and cluttered.
RuleStatementSurroundWithBraces	Recommendation	An unsurrounded if or loop statement introduces ambiguity and the possibility of an error into the code. For example, if a line below the statement is removed, then the next line below it becomes part of the statement, which may not be the intent. Also, statements without curly braces rely on indentation to communicate the meaning. It is better to be explicit and demonstrate the intent by surrounding all such statements with curly braces.
RuleStatementEmptyStatement	Recommendation	An empty statement is an extraneous piece of code. Such statements make code more difficult to understand and maintain. Empty statements typically arise as a result of accidental code deletions.
RuleSwitchDefaultLabel	Recommendation	A missing default label in a switch statement makes the code incomplete because the intended behavior is not clear. Providing the default label results in more explicit and robust code.
RuleSwitchBranch	Recommendation	The switch statement is meant to be used to avoid a lot of nested if/else constructs. However, when there are just two choices, using a switch statement is unnecessary and unclear.
yield	Recommendation	Thread.yield() is meant to suspend the execution of the current thread and let the other active threads run. Yield is equivalent to Thread.sleep( 0 ). Even though the yield() is guaranteed to work, there is no guarantee that another thread will not be suspended again. This is because the task intended by yield() call may not be fully accomplished before the control is returned to the yielding thread. When designing threading behavior in complex, large-scale programs it is not recommended to use yield to ensure the order of thread execution. The lock-based wait()/notify() approach is preferred instead.
extendthread	Recommendation	Extending Thread vs. implementing Runnable problem is an example of the Inheritance vs. Delegation Principle. In most cases, delegation is preferred to inheritance. Here as well, there is no good reason to extend Thread class. Instead, it is cleaner to implement Runnable interface and instantiate Thread to run it.
synchronizedthis	Recommendation	Synchronization problems are difficult to identify and debug, so it is critical to consider what exactly is being locked and why. Creating separate locks to protect different data sets is a good practice that will lead to improved performance and clarity. Using this as a lock is not recommended because it is also used as an implicit lock when the method is declared synchronized.
notifyall	Recommendation	The difference between java.lang.Object.notify() and java.lang.Object.notifyAll() is that the first method tells JVM to inform the first thread that is waiting on a lock, while the second tells JVM to inform all the threads that are waiting on a lock. Since the number of waiting threads is often difficult to anticipate, it is recommended to call notifyAll() instead of notify().
waitoutsideloop	Recommendation	The call to wait() suspends the execution of the calling thread until another thread notifies it that the intended state of the program is achieved. The wait() should occur based on the termination condition. It is highly recommended that the wait() call is made inside of the while loop, because another thread may invoke notify even if the condition on which the current thread is waiting is not met
synchronizedmethod	Recommendation	Synchronized method implicitly uses 'this' as a lock to prevent other methods and blocks with the same lock from parallel execution. There are several reasons why this is undesirable. Most important is that since synchronized blocks are computationally expensive, it is desirable to place as less code as possible inside of these blocks. If the method that is declared as synchronized is operating on more data that needs to be protected then such method is unnecessarily expensive. Another reason is that some programmers are not aware that 'this' is used as a lock when method is declared synchronized, so it is better to be explicit
whilesleep	Recommendation	The sleep() call causes the currently executing thread to cease execution for the specified number of milliseconds. The thread does not, however, loose the ownership of any of its monitors (locks). Calling sleep inside of the while loop is known as a busy-wait, since it does not allow other threads to operate on the shared data.