10 September, 2020
The software development process consists of many activities – one of them (and in my opinion, one of the most problematic) is the code review process. Reviewing the code means one developer verifies the quality of code provided by another developer. It’s a tiny thing that if done right will benefit the developers and the project alike. If neglected, however – it can stress people out, cause tension in the team and even ruin the entire software product. I’ve been reviewing and have been reviewed (with various outcomes) – here’s what I’ve learnt and summarised as some of my personal code review best practices.
How to do code reviews?
Many young programmers are told to perform code reviews with no preparation, nor explaining how to approach it. They are thrown into the deep end and don’t know how much they can afford to comment on the code created by more experienced colleagues. They end up approving the pull requests after a single scroll, assuming that if the changes were made by a person with more seniority, they are probably right. After all, it can be hard to correct somebody more experienced than you.
On the other hand, we have experienced programmers who, not consciously I hope, torture programming adepts with their code reviews. Seniors tend to comment on bugs in a way that can really stress out the author of the changes. They can even add opinions that create confusion instead of helping. As a result, code review becomes something that people are afraid of, avoid or conduct it superficially.
And I’m not making all this stuff up. I have come across all of this (and much worse) during my many years of a programming career. Therefore, in this article, I would like to share with you some tips and the best practices for code review that might convince your team members to trust you with reviewing code they wrote.
1. Instead of giving orders, make suggestions
When you see a defect in the source code during code review, it often happens that a better solution immediately pops to your mind. You may then be tempted to unceremoniously write it in a comment and indicate to the author of the pull requests how they should correct it in a certain way.
“Use the mapping object, break this function into several others, rename this variable! Yes, now!” This “respect my authority” approach may tempt you, especially if you have much more seniority than the person whose code you are reviewing.
However, when writing a comment, do you think about how it will be received or read?
The author of the pull request will make the change, but the comment itself may be undermining that person – like a patronising slap in the face, pointing to an “obvious” error.
Obviously, everything you write in the comments can be misinterpreted. It is important to make an effort and try to compose your message in a way that has a friendly overtone. You don’t want to be misunderstood or ruin somebody’s day with an imprecise message. If you want people to perceive your messages right, you need to remember the ground rule:
You’re working to perfect the code together, not issue orders around like generals to their privates.
Useful phrases for code review:
- I think it would be better to use a mapper here.
- This function has gotten long, maybe it would be worth breaking it down?
- What do you think about my proposal to rename this variable to…?
As a result of this approach, you may need a little more time to write your comments, but then your code review will feel like a conversation and cooperation between two developers who mostly care about the project’s quality. The bottom line is: the whole point of your comments is not pressing somebody against the wall but politely pointing out things that you think could be improved.
Thanks to this, the newbie programmer feels that there is a place for open discussion and civilised exchange of arguments. It may even encourage them to explain their reasoning and maybe change your mind about something.
Remember – give suggestions and constructive feedback, not authoritative orders.
2. For better code, don’t write “what” without “why”
In programming, we rarely come across problems that can only be solved in one, specific way. Your comments in code review result from the experience or knowledge you have gained. So when suggesting changes to the code, it’s usually good to write not only what you want to be corrected, but also why you think so. It is about the simple argumentation of your position.
Spend some time justifying your opinion, especially if the suggested change results from your personal preferences or company standards that a new employee may have forgotten or not known about.
Useful phrases for code review:
- I think a mapping object could be used here for general code readability.
- This function has gotten way too long, so maybe think about moving this block of code into a new function to reduce it.
- I suggest changing the name of the variable items to filteredBooks so that you know what exactly is in this variable. What do you think?
The advantage of this behaviour is that you solve several problems at once. The person on the other side will know why they are doing something, and in the future, the clear picture will allow them to do similar tasks much faster (and possibly pass that knowledge forward). And it saves your time too because once you’ve explained yourself, people won’t come to ask you what you mean exactly.
And most importantly: by explaining what, how and why there is a chance that the author of the pull request may learn something new and in the future, they will know what to look for in code reviews themselves. That prevents them from repeating the same mistake again because now they will fully understand the consequences. You must agree, that definitely benefits the entire team!
3. Make sure to sometimes say something nice during your code reviews
Although good code review is about pointing out imperfections in the code, your comments don’t have to be limited only to finding errors. When conducting reviews, you can also use comments to praise someone’s good work. Do it especially if you have learned something from this code or if you noticed a solution you wouldn’t have come up with yourself.
Useful phrases for code review:
- Wow, a nice way to shorten this condition.
- It’s great that you have separated this code to a new component, it will be handy when we will start working on other features.
- Nice trick, I didn’t know that was possible.
Of course, compliments should be properly dosed so as not to lose what is most important in code review. One extra comment with a positive overtone won’t cause anyone a problem, though. And it will certainly please the other programmer and prove that you made an effort to check the commits.
This advice is not only for more experienced programmers who check the code of their junior colleagues and want to write something motivating.
When you check the pull request of someone with more seniority, you can also write a positive comment to let them know that you found something that made an impression on you.
After all, it’s you doing the review and it’s about your code reading experience, right? Everyone likes to hear kind words, and appreciating someone’s work is a proven way to win over people.
4. Don’t be spiteful over mistakes when reviewing code
Everyone makes mistakes. It’s a truism, but it is very noticeable in software development. It doesn’t matter whether it’s a beginner or a very experienced pro, everyone does stupid things in their code. It may be caused by a bad day, overload with duties, lack of domain knowledge or ordinary carelessness. Eventually, everyone stumbles over something and throws the most ridiculous things into their commit – logging in to the user’s screen, no edge-case check or something that works correctly only on even days of the month. We’ve all seen things.
When doing code review, try not to show your disappointment, dissatisfaction, anger or impatience. Even with a sad emoji. After all, pull requests and review processes are for finding errors.
Phrases you SHOULDN’T use in code review:
- Again? It’s the same mistake as the last time. 🤬
- Did you even test this? I don’t think it’ll work.
- Could you try a little harder to come up with better names for the methods?
What if someone constantly makes the same mistakes in their code or code review? My code review best practices encourage you to consider where the core problem lies. Lack of work experience? Being new to the project? Lack of communication in the team? Pull requests are definitely not the place to solve such problems.
It’s much better to approach that person and talk face-to-face. Ultimately, bring it to a project manager’s attention. Try to make your comments on the source code professional, without negative emotions. It’s not about ruining someone’s day.
Remember that code review concerns lines of code, not a human behind them.
So any comments regarding how much the code’s author screwed up are unnecessary and will only cause damage to the whole team. Remember that supervisors have access to the repository service so you being spiteful in code reviews comments will look like a punishment for the author in front of the bosses and colleagues.
From my experience, direct personal attacks in code review are a rather marginal case (or I would love to believe that), but I’m more concerned with tiny manifestations of dissatisfaction with the imperfect code. Mean comments, angry emojis, passive-aggressive tone or counting how many times someone has made the same mistake ARE NOT appropriate code review standards.
This rule also applies when you review the code of your best buddy who will definitely not be offended with your mean inside jokes. There’s always a chance that someone else from the team will see these code review comments. People react differently to such remarks, so it’s better not to take risks and keep it to yourself during any coding review.
Just be nice to each other. Always.
💡 Read more: Dear developer, why does your Project Manager suck?
5. Don’t be afraid to comment during code reviewing
Do you know what’s the worst about receiving code reviews? When your pull requests are approved without any comments, but then bugs are found in your code anyway (even as trivial as the commented line of code being left behind). You might think that someone skimmed through the code review, scrolled the entire pull request down and immediately clicked the “Approve” button.
Unfortunately, sometimes you couldn’t be further from the truth. Someone may have picked the bug up but chose not to leave a comment. Why? The reasons vary – the offence was too “trivial” to be dealt with, or the reviewer wasn’t sure if something was a mistake and ignored it.
Sometimes it’s all about seniority. If a much more experienced person wrote the code, you might think that they are definitely right and you’re wrong. On the other hand, seniors tend to let beginners slide with some issues because they don’t have the required skills yet. You cannot give up writing comments just because someone is on a different level of experience than you. If you do that, nobody will learn anything new – neither you nor the author of code. You need to point out the errors to make sure that what seems to be a defect really is one.
In the end, it really doesn’t matter what happened because all of the aforementioned reasons are wrong and potentially harmful to the entire project in the long run. Code review is not a work of one person but a collective effort and cooperation of the code’s author and the reviewer in order to achieve the highest code quality.
It’s important to communicate and explain why certain things were written this way. You don’t know if it’s good or bad? Code review best practices simply say – ask what the author of the code meant.
Useful phrases for code review:
- This part of the code is a bit problematic for me. Could you explain what it does?
- Was there a reason for writing this logic from scratch? Maybe there’s a ready-to-use, a battle-tested library that does the same thing.
- Are you sure our client wants it to work like this? As far as I remember they were talking about something else.
It’s a win-win. Either you’ve really come across something serious, or you will learn something new right away.
Remember that there are no trivial mistakes. For example, a dead code snippet won’t ruin the app’s user experience, but it can make the code maintenance harder. Someone will come across this fragment and will be wasting time on finding out what it is for, just to discover that it absolutely does nothing. When enough such trifles accumulate, the application becomes difficult to develop, and this translates into maintenance costs.
Code review best practices for better code quality
Just like people, pull requests are very different. Some add a feature that has been created for three weeks, others correct minor typos. For this reason, it’s impossible to create universal rules and the best code review practices that apply everywhere and ensure that each code reviewing goes smoothly.
However, I hope that my tips on how to review code will encourage you to strive for a stress-free and developer-friendly code review. It really brings tangible benefits to all team members – we cooperate not only to create high-quality, good code but also to inspire self-development.
Let that sink in – code review best practices first and foremost include teamwork between authors and reviewers to improve the merged code.
Under no circumstances should the code review process become stressful or confusing, or merely a formality reduced to clicking the “Approve” button. In that case, code review effectiveness suffers, the code review process loses its sense and becomes just an empty convention. And that’s always a waste of time.