In larger software applications, developer and reviewer must focus on some points to make  code easy to read, easy to understand, easy to maintain, reliable, secure and scalable. Developer should also follow the software quality assurance by using standard application design architecture, design principles and design patterns. That will help others developers to understand the code easily for future development and enhancements.

I have divided below check list in two levels, same need to follow by java developer and code reviewer to match the code quality assurance.

• Basic Code Review Check List
• Detailed Code Review Check List

Basic Code Review Check List

While initial code review first impression comes from below points then reviewer will understand the code easily and that will help for detailed review of code.

• Is code clean and easy to understand?
• Is code is following coding guidelines & standards?
• Is functions or class are too big? If yes also having lots of responsibilities?
• Is the same code duplicate?
• Can I debug/ unit test easily to find the root cause of issue/defect?

Detailed Code Review Check List

The following aspects give details idea about what need to consider while reviewing code. Some of the points are not easy to identify in code for beginner reviewer because that come from experience and work on multiple application/software.

1. Clean Code

Code should be easy to read and understandable there are some standard follow in some organizations to make the code maintainable.

10-50-100 Rule

There is very simple rule to avoid Monolithic code, Spaghetti code and make maintainable.

• 10: No package can have more than 10 classes
• 50: No method can have more than 50 lines of code.
• 500: No class can have more than 500 lines of code.

Code Formatting Style

Use standard code template/ code style and shared to all developers in team. So that every individual developer aligned properly while merging and checkout code from repository will not create any conflict. Keep below points in mind while writing code :

• Use alignments (left margins), white spaces and starting and ending points of blocks easily understandable.
• Code should be fit in standard 14 inch laptop screen so no need to scroll horizontally to view the code.
• While code review remove the commented code that can be taken again from GIT/SVN repositories if required.

We can use CheckStyle tool with maven also for reducing this manual effort and share same with all developer in team to follow same standard.

Naming convention

Naming convention for package, classes, methods and variables make code easily understandable:

• Proper naming convention(Pascal, CamelCase etc.) while deciding name for variable, classes, methods.
• Use descriptive and meaningful variable, method and class names so that not relying too much on comments.

For Ex: Method : calculateTax(BigDecimal amount) ; Variable: totalAmount; Class: CustomerAccount.java

Apart from above points there are some more which will make our code more clean and easily readable.

• Class and functions should be small and focus on doing one thing. If there is duplicate code in multiple functions create new method for same and reuse it .
• Function should not take too much parameters. If need to pass more parameters from same object better pass reference of object.
• Declare the variable with smallest possible scope.
• Don’t preserve or create variables that not use again.

2. Architecture/Design

While implementing functionality keep in mind to follow OOPS concepts (A PIE), SOLID design principles, Don’t repeat yourself (DRY) and keep it simple (KISS) . These OOPS concept and principles accomplish “Low Coupling” and “High cohesion“.

OOPS Concept (A PIE)

• Abstraction
• Polymorphism
• Inheritance
• Encapsulation

SOLID Class Design Principles

• Single Responsibility Principle : A class should have one and only one responsibility. If class is performing more than one task, it leads to confusion.
• Open & Close Principle : The developers should focus more on extending the software entities rather than modifying them.
• Liskov Substitution Principle : It should be possible to substitute the derived class with base class.
• Interface Segregation Principle : It’s same as Single Responsibility Principle but applicable to interfaces. Each interface should be responsible for a specific task and should not have methods which he/she doesn’t need.
• Dependency Inversion Principle : Depend upon Abstractions- but not on concretions. This means that each module should be separated from other using an abstract layer which binds them together.

Design Patterns

Design patterns provide standard try and tested approach to handle certain cases. They provide standard terminology which makes developers to collaborate and easier to communicate to each other over globe.

3. Documentation/Comments

• There should be an explanation for any code that is commented out.
• All classes and methods should contain a descriptive JavaDoc comment.
• All methods should contain brief comments describing unobvious code fragments
• All class files should contain a copyright header.
• All class files should contain class comments, including author name.
• All methods should contain comments that specify input parameters.
• All methods should contain a comment that specifies possible return values.
• Complex algorithms should be thoroughly commented.
• Comment all variables that are not self-describing.
• Static variables should describe why they are declared static.
• Code that has been optimized or modified to “work around” an issue should be thoroughly commented, so as to avoid confusion and re-introduction of bugs.
• Code that has been “commented out” should be explained or removed.
• Code that needs to be reworked should have a TODO comment and a clear explanation of what needs to be done.
• When in doubt, comment.

4. Logging/Debugging

• Logging for different level should be configurable.
• Log every transactions or the ones that require logging.
• Use appropriate log level corresponding to messages. For Ex: ERROR for exception.
• Always log execution time of method to check performance.

Check below link for info on logging.

Log4j2 Java Logging Example Tutorial – XML Configuration, Severity Levels, Formatting and Appenders

5. Exception Handling

• Use exceptions as opposed to return codes.
• Code should handle exceptions, not just log them.
• Catching general exceptions is commonly regarded as “bad practice”.
• Some method in the call stack needs to handle the exception, so that we don’t display that exception stacktrace to the end user.
• Exception handling should be consistent throughout the system.
• Don’t ignore or suppress exceptions. Standardize the use of checked and unchecked exceptions. Throw exceptions early and catch them late.
• We need to expand our notion of Exception Handling Conventions.
• When method return reference object always check for null before use that. For Ex:

Emloyee employee= Context.getEmployeeService().getEmployee(employeeId);

employee.getAddress().getStreet();

• There should be no catch blocks which catches an exception and throw that again. This is because the exception can bubble up to the top function call stack automatically.
• Never ever throw a general exception with a custom message. Always try to create a custom exception so that all the other code can handle this situation correctly.
• Make sure that the top most code handles all the exceptions correctly shows correct / understandable error messages to the user.
• In the case of a system crash never ever put up the error information that exposes the internal behaviour of the system.

6. Security

• Don’t log sensitive data. For Ex: Password, credit card number, CVV etc.
• Don’t throw exception with sensitive information like file paths, server names, host names etc.
• Connect to other systems securely, i.e., use HTTPS instead of HTTP where possible.
• Service methods should have an @Authorize annotation on them.
• Use prepared statements instead of statement to prevent SQL injection attack.
• Release resources (Streams, Connections etc.) to prevent denial of service attack (DoS) and resource leak issues.
• Follow Security best practices for SSL, encryption of session, sensitive data, authentication and authorization etc.
• Passwords should not be stored in the code. In fact, we have adopted a policy in which we store passwords in runtime properties files.
• Clearly document security related information.

7. Performance

• Reuse objects by using caching or Flyweight Design Pattern.
• SQL queries and joins not proper use can impacts performance. For more check JDBC Coding Best Practices
• Backtracking of regular expression impact performance.
• Use appropriate Collections classes as per requirement.
• Inefficient Java coding , algorithm and data structure in frequently executed methods leading to death by thousand cuts.
• Presence of long lived objects like ThreaLocal and static variables holding references to lots of short lived objects.
• Avoid creating unnecessary objects.
• Beware the performance of string concatenation. Use StringBuffer or StringBuilder when need to apply operation on String.

8. Concurrency

• Avoid excessive synchronization.
• Write thread-safe code with proper synchronization and use of immutable objects.
• Keep synchronization section small and favor the use of the new concurrency libraries to prevent excessive synchronization.
• Avoid calling synchronized methods within synchronized methods.
• If objects can be accessed by multiple threads at one time, code altering global variables (static variables) should be enclosed using a synchronization mechanism (synchronized).
• In general, controllers / servlets should not use static variables.
• Write access to static variable should be synchronized, but not read access.
• Even if servlets/controllers are thread-safe, multiple threads can access HttpSession attributes at the same time, so be careful when writing to the session.
• Use the volatile keyword to warn that compiler that threads may change an instance or class variable – tells compiler not to cache values in register.
• Release locks in the order they were obtained to avoid deadlock scenarios.

9. Detach Resource After Usage

• Resources that are not automatically released after usages are freed. Connections, Files, ports are closed properly..

10. On Demand Resource Delivery

• Resources are fetched and delivered only on demand. Necessary options are available for dealing with huge data such as paginations, etc.

11. No Warning/ Console Logs

• No compiler warnings should arise while running the application.
• Logs that are used while developing are cleared and none of the application information (especially sensitive ones) are written in the browser console.

12. Unit Testing/ JUnit

• Never allow a unit test that is written to show 100% coverage and doesn’t do anything that unit test is supposed to do.
• Ensure unit/mock test write properly and able to run independently of other.
• Set up should not be too complicated.
• Mock out external states and services that you are not asserting. For example, retrieving data from a database.
• Avoid unnecessary assertions.
• Start with functions that have the fewest dependencies, and work your way up.
• Write unit tests for negative scenarios like throwing exceptions, negative values, null values, etc.
• Don’t have try/catch inside unit tests. Use throws Exception statement in test case declaration itself.
• Don’t have ant System.out.println(…..)
• Always try to have a unit test for the new piece of code. In an ideal condition, we should have 100% unit test coverage.
• Make sure the JUnit test covers all possible values.

To Learn more on Junit and Mockito follow Mockito + Junit Tutorial

13. Framework & Libraries

• Favor using well proven frameworks and libraries. For Ex: Spring Libraries, Hibernate Libraries, Google Libraries etc.
• Legal use of third party libraries. If using third party libraries that should approved from admin and licensed. For Ex: Oracle.

14. Configurable Items

• Keep environment specific properties like password, directory location in database configuration table.
• Password should store in encrypted form.
• Keep hardcoded values like url, service endpoints in properties file.

15. General Programming Practices

• No syntax/runtime errors and warnings in the code.
• No deprecated functions should use in code.
• No public class attributes.
• Always try to initialize the variable before using that in a function.
• Use class final and object immutable where possible because immutable objects are thread safe and secure. For Ex: String Class.
• Always try to use constants in the left-hand side of the comparison. That is instead of doing if ($variable == “Saurabh”) always use if (“Saurabh” == $variable) because this will help to identify the errors in the earlier stage of development even if we miss and “=” from that statement.
• Check that each function is doing only a single thing. That is a function named createEmployee should never delete the existing employee and create it again.
• Always try to separate out the code with view. Ideally the view/template should be logic free.
• Optimizations may often make code harder to read and more likely to contain bugs. Such optimizations should be avoided unless a need has been identified.
• Always have an eye on the recursive functions and make sure it will have closing condition.
• “Dead Code” should be removed. If it is a temporary hack, it should be identified as such. Check if code has file/class/function level comments. And each of this comments should explain what the file/class/function is doing inside it.
• No magic numbers and hard coded value. This should be defined as a constant well commented about the purpose.
• Never allow bad code with some good comments.
• Always commit /Rollback database transaction at the earliest. Keep the database transaction short as possible.
• When use serialization of object use Serialization ID.
• Use appropriate collections as per requirement.
• Prefer return empty collection instead of Null.
• Use equals over ==.
• Use Array when number of element fixed while ArrayList when elements vary.
• Always check null on reference object before use it.
• When use String search on some text. then match will return index otherwise as -1.

Tools for Code Reviews

Below are some tools to do code analysis on static code for improving code quality. These tools run thoroughly on the entire project and generate reports. Some tools are for backtrack of review comments.

• Use the tools (based on technology) such as SonarQube, NDepend, FxCop, TFS code analysis rules.
• Use plug-ins such as Resharper, which suggests the best practices in Visual studio.
• To track the code review comments use the tools like Crucible, Bitbucketand TFS code review process.
• Use the tool PMD to identify possible bug, dead code, overcomplicated expression, suboptimal code, duplicate code, etc.

Conclusion

The objective of above code review checklist is to introduce and apply concepts and provides a direction to the code reviewer to conduct effective code reviews and deliver good quality code. Initially it take time and required bit practice to check code from different aspects to make you expert code reviewer.