Code reviews — tips and good practices for the reviewer

The problem

Speaking from experience, I can say that one of the most disliked activity by developers is the code review process.
This is for a number of reasons.

1. Time — when you are going over someone else’s code, it’s hard to know how long it will take. I remember code reviews I had with over 150 modified classes, or ones that modified a class that had over 4,000 lines of code.
2. Context switch — the task for a code review rarely comes when you have time for it, meaning you need to stop whatever it is you’re working on and make time for it.
3. Difference of opinion — since there’s usually more than one solution to every problem, it’s hard to go over a solution you may not agree with its approach and comment on it.
4. Code style — in most of the places I worked with including the current one, the lack of definitive code style makes it harder to review code in a style that is different than yours and regarding how much to comment on it in the code review.
5. Lack of Documentation — when going over the code, there’s usually no UML/class diagram or documentation added to it, meaning it’s up to you to understand the logic that was done.

Despite this, I think that code review tasks are a substantial part of what makes a good developer a great one. It gives you insight to other developers way of thinking and being a part of coding tasks you do not have time to write it. Ultimately performing code reviews is a great way to increase your knowledge base, share your input and be able to assist other developers without actually writing the code yourself.

First, I’ll share some code guidelines for the code reviewer I use at my work and ones I see that others in the team use in order to make the code reviews be less of a nuisance and more of a jumping point to improve yourself as a developer.

Guidelines

1. Dedicated time — obviously there are varying lengths of code reviews, from ones that can take less than 30 minutes to ones that can take days (150 classes, remember). When working either in Sprints or Kanban, you can approximate how long during a week you need to spend on code reviews (if you’re not sure, just write it down, during a month for every week and take the average time). Let’s say it’s 5 hours a week.
   Put 1 hour every day at a time you think fits your schedule to do code reviews and use that hour for the code reviews. Obviously there are days when that duration can go longer or shorter, but it will enable you to have dedicated time for it during the week.
2. Prerequisites — this might seem small but having a checklist of what is required to send a code review can make the process so much easier for the developer doing the review. This list can vary from one work place to another, but a couple of examples are adding the ticket requirement to the Pull request, running automation on the code, running tools like linter to fix synatx issues, performing internal testing, writing unit tests for it and running the code itself. One way to implement this is to add a default template to the code review with a checklist of what was done prior to sending it.
3. Avoid Ping pong comments — I saw many cases when developers got into heated arguments in the code reviews that resulted from their way of thinking, their code style or sentiments on code that was initially written by them. Instead, the comments should be based in logic and not emotions and to give sound reasoning to your arguments. If you see that the comments turn into more of an argument, I suggest to set a short meeting to discuss those specifically.

Techniques

Other than general guidelines, I also want to share techniques for the reviewer that will make the code review even better.

1. Go over the requirements — you’ll be surprised how often this is overlooked. In many cases due to duplicated requirements or ones that were modified since the original ones were written, the solution often strays from them and in some cases code was added that is unrelated to the requirements at all. This should be the starting point of the code review.
2. Comment only on the changes in the code review — in so many times, I saw developers comment in the code review on changes unrelated to the pull request. This is often the reason the code review stretches beyond the estimated time. I know it’s tempting but even if you see code in the pull request that is wrong in your opinion, but unrelated to the changes the developer did in the pull request, do not comment it within the scope of the code review. Regarding how you can handle these issues, there are many options here I will not get into.
3. Run the code — in many cases, it’s not enough to simply go over the code. Only when running it and looking at logs surrounding the flow, you will discover issues. In some cases, running the code can require a special configuration so this needs to be part of the code review handover.
4. Refactor vs Code maintenance — the balance here is delicate and when going over the code, you need to think in which cases it was better for the developer to write duplicated code that won’t endanger the existing code and in which cases a refactor is the better way to go. This is never black and white, there are many shades of grey here that are affected by the time the developer has to approve the code, the way the code can be used in other usages and the dangers of regression.
5. Logic over syntax — comments on code reviews usually differentiate between the code style and syntax of the code and the logic that the code implemented. Find the right balance here. I prefer taking the approach of writing only logic based comments and if there’s anything related to the syntax, I either reference our coding style definitions in the team or raise it in the developer meetings.
6. Readability over efficiency — in some cases it’s better to write code that is longer if it’s more readable and will make it easier for other people to understand it and refactor it at a later time. I saw many cases where a super efficient code was written in a way that caused other developers confusion and later was refactored with bugs due to its implementation. Another option is to add comments to the code if there’s reason to believe the comment will help understand the implementation.

Conclusion

Even though code reviews may seem exhausting, never-ending and hard for you to find the time for them or approximate how long they will take, when taking the right approach to them, it will be much easier to handle them.
You will see that the better you get, more people will come to you for assistance in reviewing their code, you will be in a great position to support your team members and for them to support you in return.