Java Code Review Checklist

Code reviewing is a good practice and helps in improving the code quality and avoid un-neccessary bugs resulting from poor coding practices. When a experienced person is reviewing the code then there are a number of check points which should be cross checked. By performing code review, we not improve the quality of deliverables, but also improve our relationships with peers. Many a times, when doing a review, you come across pitfalls which can make you humiliated if sent to production.

Agreed that code reviews require extra time after you have put in efforts to write the code, but if your aim is high quality for the code, you should proactively seek comments on your code and improve the same. The best part of having reviews is that once you fix an issue with the code, there are very less chances that you will repeat the same mistake again.

This post lists the points which should be used for reviewing the code hence it can act as code review cheat sheet.

The following points should be present in every code review guidelines and can act as best practices list for Java applications:

1.  Documentation
2.  Functionality
3.  Security
4.  Performance
5.  Coding practices
6.  References

Documentation

1) Javadoc comments should be added to the class as well as methods.
2) In case of support projects, add bug id’s to track changes made for a bug.
3) Check that explanation for quick hacks or complex logic is present as one is bound to forget them.
4) For publicly available code, make sure copyright lines are added at the top of each file.
5) HTML, JavaScript, CSS and script files should contain documentation for complex parts.

Functionality

1) If similar logic is being used at multiple places then it should be declared in a helper class and called from multiple places.
2) Usage of API classes and methods should be encouraged instead of writing custom code for performing the same operations.
3) Extra emphasis should be given on writing the unit test cases for the code which is going to be released.
4) The addition of any piece of code should not break existing functionality.
5) In case of web applications, JSP’s shouldn’t include Java code but should make use of JSTL, EL or custom tags.

Security

1) Any part of code should not try to run the user supplied input without escaping it. This usually includes executing eval function in JavaScript or executing SQL queries entered by user.
2) Block IP address which submit a lot of requests in a short span of time. These kind of attacks can bring the system down by consuming the bandwidth.
3) Each class, variable and method should be assigned an access specifier suitable for its use.
4) Check if the solution can be implemented without the use of iframes as they are linked to security issues.

Performance

1) All database and file handlers should be properly closed when there is no further need for them.
2) Usually a single database transaction can be done by writing the SQL query in multiple ways and there is a huge difference in the performance of database transactions depending upon the way in which SQL query is written.
3) Creating immutable class should be encouraged than mutable classes.
4) Usually we come across situations where similar logic has to be executed for different scenarios with minor differences. In this case, try using the same code with if else condition to take care of special handling.
5) Check that the developer is not making heavy objects where the purpose could have been solved by light weight objects.
6) In case of web applications, make sure the developer has used optimum image sizes, CSS sprites and browser caching for static resources.
7) If some information could be required at any time when the application is running, then store it in application context.

Coding practices

1) Unused member variables should not be present in the classes.
2) Proper catch blocks should be added for exception handling instead of single Exception object handler.
3) Proper naming conventions should be used for variables, method and class names.
4) Instead of using hard coded strings, constants should be declared in a separate Constants class.
5) No trailing spaces should be present in code lines.
6) Uniform coding standards for braces, loops, if-else, switch should be used across the application.
7) A single method should not exceed 100 lines of code as it becomes difficult to maintain beyond that. Split a single big method into multiple smaller methods.
8) A single statement should not go beyond the viewable area of the editor or IDE and should be split across multiple lines.
9) The best way to check if the String object is neither null nor empty string is to use the following code:
if(“”.equals(str))
10) If a class has many member variables and the instance of that class can be initialized by initializing only a partial number of variables then it is better to have static factory methods for initializing the member variables instead of overloading the constructors.
11) Add appropriate access specifiers to methods instead of marking all methods in a class as public.
12) Follow best practices suggested by any framework/library being used in the application like Spring, Struts, Hibernate, jQuery.

In addition to the above listed common code review points, it also important to write unit tests for your code and submit the same for review. Unit tests help in regression testing and hence save a lot of time.

Some of the above code review tasks can be accomplished by using static code analysis tools. These include CheckStyle, FindBugs andJTest. These tools get configured in the IDE as a plugin and can be run on the code.

References
1. Java Code review checklist