What is Code Review?
Code Review is the examination or the analysis of the software code before deploying it in the development environment. For many of you Code Review is nothing but the PR (pull-request) request which we raise when we are done with the functional changes and want the feature to be merged into the master branch. But Code Review is more than that, It provides an extra set of eyes to evaluate the code for functional or technical issues. It helps the team to deliver stable and quality code and at the same time enables exchanging knowledge amongst the team members thereby improving the developer skills.
The typical Code Review process
Developer(Author) codes the required features and raises a PR request to review the changes done in the project
Reviewers evaluate the code on different parameters and add comments/annotate to the respective line numbers.
The author based on the comments either modifies the code to take that comment into account or if he/she does not agree with that they can reject it with convincing arguments In some cases the discussion can be done offline to resolve conflict
The whole process is repeated for any piece of code change that we want to deliver.
The above steps are the most common and widely used Code Review method, but there are other approaches like pair programming or over the should review.
When it comes to code pairing, review is done at the time of coding itself; it's the responsibility of both the person to inspect each other's code and suggest improvements. In code pairing as the code is already reviewed we can skip the PR review step. We in mavericks follow code pairing as the feedback loop is tighter In contrast to PR review wherein the review is done when we are done with the feature and code is ready for deployment.
What things to look for in Code Review?
We have seen what Code Review is and what are the steps involved Let's go through some of the important things that one must look for while reviewing the code.
Functionality
These are the first things that any reviewer should start with. Below are the points that are to be considered in the functionality review
Does the code implements the functionality that is intended
Are all the uses cases fully implemented?
What about the edge cases are they handled well.
Are they any potential side effects to the existing functionality
If you are not sure of the functionality or something is ambiguous ask the author to give a demo or try it yourself it will also help us in identifying if any configuration or file is missed in the commit.
Design
Does the code conforms to the existing architectural design/patterns
In the case of object-oriented programming are we following the SOLID design principles
Is the code extensible
Is the code over-engineered i.e. it is more generic than it is required
Coding Conventions
Coding conventions are a set of styles, guidelines, or best practices that are predefined for the project. Some of these practices are common across language and some are project-specific. In this section, the reviewer must look for
Is the new code adhere to the defined standard and guidelines
Code is well-formatted
Proper naming conventions are followed
Does not contain commented code
No hard coding use constants/configuration
Error Handling
Exceptions should be handled like invalid inputs, web service failure issues, etc.
Errors are written to the logs for debugging purposes
Maintainability
The code should require the least amount of time to support in the future. Following are the things one must evaluate during Code Review
Readability: The code should be less complex and self-explanatory. If it is taking more time to understand then it needs to be refactored.
Unit test: Unit tests are written and covers all the scenarios (positive as well as negative)
Loggers: Loggers are added and are meaningful
The DRY principle is followed
Security
Security is one of the most overlooked areas of coding. In Code Review, this is also an important factor that one must inspect. Some of the points are
Encrypting the sensitive data
Cross-site scripting
Unauthorize access
Usage of vulnerable libraries
Each of the parameters mentioned here does not cover the complete list of things but just gives a brief overview of topics that one must focus on while reviewing the code.
Any Feedback/Suggestions drop it in the comments section.