Code Review Checklist

Introduction

I tried to improve my code review skill. Reviewing 10-15 files with around 100 lines of code at a same time was bit boring and overwhelming .A bit of challenge in understanding other’s code .It’s like reading other’s brain.

So what i did was …

Execute the Code in your IDE :

First I downloaded the code (code review code) into my intellij (IDE),I ran the code and unit tests. While running the unit tests,I got some idea about what exactly the code does.

Then I come up with the checklist.

My code review checklist points :

• Clarity or code Readability 
• Correctness of the code
• Verifying the Design 
• Exceptions and Failure Handling 
•  How easy to maintain that code

1.Clarity and Code Readability :

1. Is the code is easy to read ? Are you able to understand each line and function does at a glance ?
2. Each method or function content is related to the name of the method or function ?
3. Number of lines are limited ?

Appropriate comments added for the method ? For example If a piece of code is removed , they should add a comment why that is removed ?

2.Correctness of the code :

1. Are there any bugs in handling loops or conditions ?
2. Enough unit tests added for each scenario ?
3. Is the code  thread safe ?
4. Appropriate data structure used ?
5. Appropriate decision making syntax used ? 
Decision or condition logics can be written using "switch case", "if conditions" ,"loops".
For Example if the developer used Switch case, we need to verify that 
"switch case " is  appropriate for that scenario ?

3. Verifying the Design :

1.  Is the design fits within the existing design within that package or module ?
2. Is there any code duplicated ?
3. Is there any possibility to separate a piece of logic  as a separate library or utility ?
4. Is that constant values are separated in a config file or until package ?
5. If any new dependencies are added , is that required ? Is that light weight ?
6. Is there any other powerful library for the same scenario ?

4. Exceptions and failure Handling :

1. Is there enough exception handling added to handle edge cases ?
2. Is the code resilient to handle worst-case scenarios , 
    garbage return values  from any external service call ?

5. How Easy to maintain that code :

Remember , The code which you are reviewing can be used for next 10 years or 20 years :). So, It is very important to check the how easy that code is to maintain.

a).Logging :

For any Prod issues, only logs are the real source to investigate the problem. So proper log statements should be present.Breakdown the code and check through the log statements and are you able to understand the flow ?

b).Metrics :

Is there enough metrics added to monitor the services ?

c).Latency :

Is the code affecting the latency of the system ?

d).Performance :

code needs any optimization ?

Example ,

If you are making call to external service , instead of making call every time , we can cache the response .

If you are trying to fetch static data from database , you cab cache that data .

Is the code is backward compatibility and can be reused for future use case ?

Conclusion :

In our organization, we have tools like SONAR and other tools to detect the null pointer exceptions , naming conventions , redundant code blocks.And if connection object or file connections are not closed the tool will detect it. So, with manual review we have to have tools configured to get best practices.