An important process during software development is having your code reviewed by others. This allows you to get insight about your code from another perspective, which allows you to grow and learn from others. This process allows you to rectify things you may have missed or to further optimise your algorithms.
Reviewing another individual’s code also allows you to grow and learn. You get to read how a certain problem is solved and get to compare it with your own solution. More importantly, you can verify certain aspects of the code if it eases readability and maintainability of the codebase.
Given that the requester of the code review and you came from different backgrounds, you must ensure that everything you convey is not misinterpreted. Even if you grew up in the same environment, remember to be always be nice.
When to Do a Code Review?
Doing code reviews is usually only one aspect of your responsibilities. You also have other tasks that you need to complete and they may or may not have higher priority than this. Learn to weigh your tasks and see if doing the code review earlier would only take few minutes of your time and would unblock the requester.
As a rule of thumb, do the code the review as soon as you’re able.
What to Check In a Code Review?
One of the most divisive aspects that you have to check is style. Each developer has their own personal preferences on how a code should appear. Thankfully, organisations usually have style guidelines in place that everyone has to follow. This removes the arguments among developers on which style is better. A unified style allows every developer to read the codebase easier.
Alternatively to manual checks, you can get linters to automatically check if entire code follows the style guidelines.
The most important thing to check when doing code reviews is correctness. Is the code doing what the author has intended? Will it continue to work for edge-cases? Does it break other existing features?
Another thing to check is efficiency. Does the code load the data efficiently? Does it load only the necessary data? Does it still load fast given a huge dataset?
Don’t forget to check about security. Does the code allow end-users to access restricted data? Does it accidentally destroy existing data? Is it vulnerable to attacks (e.g. brute-force)? Does it expose end-users personally identifiable information (PII)?
Maintainability is another thing we have to check. Is the solution unnecessarily complicated? Does it try to solve future unknowns? Does it introduce duplicate libraries, classes, or methods?
One of the most missed things to check is documentation. Complex solutions should be explained in detail in a documentation. This allows everyone to understand the rationale behind current decisions. Documentation should also provide us instructions on how to perform different things within the application.
Working in tandem with documentation are tests. Tests are usually the best documentation on how a method should behave. Check if there are written tests in conjunction with the author’s code changes. Check if tests are properly passing and there are no false positives. Are tests failing as expected when code is broken?
Codebase maintenance is hard especially when multiple people are working on it concurrently. New people come in and read the history of the codebase, they check what other files were changed when a bug was introduced. They try to understand the author’s intention in a commit. So to make things easier for everyone, another thing for you to check is the commit history. Are the commit messages descriptive? Can the commits be squashed together? Are refactors committed separately from the main change?
These are just some things that you, as a reviewer, should check when doing code reviews. Remember that you should get the message across in a nice and concise way. If you can’t compromise on one of the issues you’ve raised, try to view the situation in his perspective and just let it pass.