What all to consider while reviewing the code?

There are various static code analysis tools available which help in improving the code quality and enforcing certain rules. There is still a need to have manual code reviews by your peers to learn and improve coding practices.

I try to check the following points while reviewing the code from my peers:

Modularity & reusability

• Is the code being repeated in multiple places? If yes, then could it be converted into a method.
• Is the method too long to understand & maintain? If yes, then looking for the possibility to break it down to multiple methods.
• Can some specific methods be moved out to helper classes, which decrease the source code in a single file and increase the reusability.

Readability

• Standard naming conventions are followed for file names, method names and variables.
• Ubiquitous language is used while naming the classes & operations instead of making it very technical.
• Comments & java docs are included.
• Formatting, indentation & spacing is appropriate.
• Code is not unnecessarily complex to understand, e.g.
• Use switch-case instead of nested if statements
• Conditions are not too large to understand

Logging

• Any sensitive PII attributes are not getting logged in plain text
• Not too much logs
• Selection of appropriate log levels
• Enough information is getting logged to debug any issue later on

Code coverage

• Enough automation test to cover the new code and its impact on the existing code
• Choice of the test — Unit test or Integration tests
• Functional scenarios are covered using the tests
• Tests are present for negative scenario e.g. Null or empty inputs, wrong configuration etc.

Error handling & retries

• Custom exception wherever applicable
• Exponential backoff retires (again wherever applicable)
• Escalating errors to different “modules/part of code” if applicable
• Catching exception and logging when the process need not be failed in case of exception

Input checks

• Sanitizing the input
• Data validation e.g. data type, format etc.
• Null checks
• No assumption on input validation in some other part of the code

Resources leak prevention

• Closing or stopping the resources (e.g. files, connections etc.) in both success and failure scenarios

Performance & data structure

• Usage of appropriate collections
• Possibility of concurrent processing
• Filter before processing to not waste the CPU cycles on processing unnecessarily