Java Code Review Checklist: How to Review Java Code

Java Code Review Checklist

How to Review Java Code? Which things should be maintained, what are the common flaws we should look after? There are thousands of questions in our mind. Here I've tried feed all those curious minds.

Preface

With time when you've become a senior programmer, you need to have a code review checklist to evaluate code quality that your fellow programmers write. On the other hand, programmers need to know quality matrix based on which their code will be measured. Here I've compiled a review checklist from various online sources that can help both engineers and tech leads to ensure better quality code in their projects.

Categories to Review

I've categorized the review checklist to improve java code by focusing on a particular area of the program or one aspect at a time.

I have categorized code review checklist into following 4 categories:

1) General

2) Documentation
3) Threading, and
4) Security

The Code Review Checklist

For each of the above categories, the following sections enlist the important review checklist items. Items in the list may be duplicated in more than one category because those are applicable in both. All the item enlisted may or may not necessary for all java project.

General

General-1	The code works
General-2	The code is easy to understand
General-3	Follows coding conventions
General-4	Names are simple and if possible short
General-5	Names are spelt correctly
General-6	Names contain units where applicable
General-7	Classes and Methods should be small!
General-8	Enums are used instead of int constants where applicable
General-9	There are no usages of 'magic numbers'
General-10	All variables are in the smallest scope possible
General-11	All class, variable, and method modifiers are correct.
General-12	There is no dead code (inaccessible at Runtime)
General-13	No code can be replaced with library functions
General-14	Required logs are present
General-15	Frivolous logs are absent
General-16	No System.out.println() or similar calls exist
General-17	No stack traces are printed
General-18	Variables are not accidentally used with null values
General-19	Variables are immutable where possible
General-20	Code is not repeated or duplicated
General-21	There is an else block for every if clause even if it is empty
General-22	No complex/long boolean expressions
General-23	No negatively named boolean variables
General-24	No empty blocks of code
General-25	Ideal data structures are used
General-26	Constructors do not accept null/none values
General-27	Collections are initialized with a specific estimated capacity
General-28	Arrays are checked for out of bound conditions
General-29	Favor the use of standard exceptions
General-30	Catch clauses are fine grained and catch specific exceptions
General-31	APIs and other public contracts check input values and fail fast
General-32	StringBuilder is used to concatenate strings
General-33	Null/None are not returned from any method
General-34	Floating point numbers are not compared for equality
General-35	Loops have a set length and correct termination conditions
General-36	Blocks of code inside loops are as small as possible
General-37	Order/index of a collection is not modified when it is being looped over
General-38	No methods with boolean parameters
General-39	No object exists longer than necessary
General-40	Design patterns if used are correctly applied
General-41	No memory leaks
General-42	Methods return early without compromising code readability
General-43	Check parameters for validity
General-44	Return empty arrays or collections, not nulls
General-45	In public classes, use accessor methods, not public fields
General-46	No use of Object class, use generics instead

Documentation

Doc-1	All methods are commented in clear language.
Doc-2	There is no commented out code
Doc-3	Comments exist and describe rationale or reasons for decisions in code
Doc-4	All public methods/interfaces/contracts are commented describing the usage
Doc-5	All unusual behavior or use case handling is described in comments
Doc-6	Data structures and units of measurement are explained
Doc-7	Document security-related information
Doc-8	Avoid excessive logs for unusual behavior
Doc-9	Do not log highly sensitive information

Threading

Thread-1	Objects accessed by multiple threads are accessed only through a lock, or synchronized methods.
Thread-2	Race conditions have been handled
Thread-3	Locks are acquired and released in the right order to prevent deadlocks, even in error handling code.
Thread-4	StringBuffer is used to concatenate strings in multi-threaded code
Thread-5	Synchronize access to shared mutable data
Thread-6	Keep Synchronized Sections Small

Security

Sec-1	All data inputs are checked (for the correct type, length/size, format, and range)
Sec-2	Invalid parameter values handled such that exceptions are not thrown
Sec-3	No sensitive information is logged or visible in a stacktrace
Sec-4	Make class final if not being used for inheritance
Sec-5	Minimize the accessibility of classes and members
Sec-6	Input into a system should be checked for valid data size and range
Sec-7	Release resources (Streams, Connections, etc) in all cases
Sec-8	Purge sensitive information from exceptions (exposing file path, internals of the system, configuration)
Sec-9	Consider purging highly sensitive from memory after use
Sec-10	Avoid dynamic SQL, use prepared statement
Sec-11	Limit the accessibility of packages,classes, interfaces, methods and fields using access control mechanism
Sec-12	Validate inputs (for valid data, size, range, boundary conditions, etc)
Sec-13	Define wrappers around native methods (do not declare a native method public)
Sec-14	Make public static fields final (to avoid caller changing the value)
Sec-15	Avoid exposing constructors of sensitive classes
Sec-16	Avoid serialization for security-sensitive classes
Sec-17	Be careful caching results of potentially privileged operations

References

Followings are the web resources which I've used to write the review checklist:

Tariq Notes

Search This Blog