Code Review - What is and why you should care
Recently, I gave a talk on Code Review at the WomentTech Global Conference (I even talk about how it all happened in this post). It was such a great experience, I decided to share in a text (and some GIFs). It ended up becoming quite a long post, so my suggestion is to pair the reading with a Chai Latte (or really anything you like to drink).
As always, feel free to talk with me about this, or really anything, on Twitter, @thamyk. Other contacts on my about page.
Story time
Before we jump into code review, let me share two stories with you.
In 1962, NASA launched the Mariner 1, a vehicle intended to perform a Venus flyby. Roughly 5 min after launch, the Range Safety Offices called for self-destruction of the rocket. This caused a loss of more than 327 million dollars.
Some years later, in 1999, NASA developed a satellite that was supposed to orbiter Mars, collecting weather information from the planet, the Mars Climate Orbiter (MCO). But the satellite never really got into Mars’ orbiter, becoming lost in space (literally). This caused a loss of 125 million dollars.
What do both of these stories have in common? Well, both have their root cause boiled down to issues in software. Issues that are commonly caught during code review. The Mariner 1 counted with the omission of a hyphen in coded computer instructions, which allowed transmission of incorrect guidance signals to the spacecraft. While the MCO had two important components communicating with each other, one considering the imperial measuring system and the second the metric one. Needless to say, recipe for disaster.
One important disclaimer here is that I’m in no way blaming developers from NASA, nor stating they don’t do code review. What I want you to take from these stories is that: software errors can get pretty costly, especially when the software is in production. And I’m not alone claiming this.
A study done by IBM estimated that the cost of an error catch after the release of the product is 30 times the cost if caught during specification.
Code Review will be able to contribute to catching errors during implementation, which helps lower the cost of the development.
What is code review?
We went a little ahead, so let’s back down and answer an important question. What is this code review I’ve been talking about?
Code Review is an activity where one or more people go over someone else’s code, proving feedbacks (negatives and positives). During this activity, the following are identified:
- Completeness: Is the problem being resolved as a whole? Any missing (and important) part?
- Consistency: Is it following the guidelines for the product? And the guidelines for the code?
- Side effects: Could this cause unwanted effects?
- Maintainability: Will this be easy to maintain? Coming back at this code one year from now, will we be able to understand it and the reasoning behind it?
- Tests: Are there enough tests for this? Are the tests covering everything?
- Performance: Could this cause performance issues? Is it optimal?
Types of review
There are many ways of code review. Here I talked about 3 types I find more common in the routine of the corporations and projects out there.
Pair programming
Pair programming is a well-known practice in our field. In it, two programmers work on developing something together, using only one workstation. During the development, possible problems are spotted and fixed.
Using this allows for fast response between peers, reducing overhead between finding an issue and fixing it. It’s great for solving complex problems, especially when each part knows only half of the domain, or even when both parts don’t know much about the problem, and together learn and work on the solution.
The downside of such a practice and one of the reasons why it cannot be used as a silver bullet is that it required the full dedication and allocation of two people.
Live review
Live review, as the same suggests, is when you call for one or more people and presenting the code on a screen, go over it, reviewing together.
Again, the upside is for this type of review is that the turnaround time between peers is reduced. The person who is reviewing can get answers from who developed as they go through the code together. Because of this, it’s a great option when the reviewer needs further clarification on the context.
One big issue with this is that it ends up forcing a context switch for reviewers, adding one more (among the many) meetings in their day. The person has to stop doing something at the given time, go to the review session, and later come back to that something that was stopped.
Asynchronous review
Maybe the most common one, the asynchronous review happens, well, asynchronously. The person sends a review request to the reviewers, and they will go over that, sending the comments back whenever they have a chance to look at it. The requester will receive such comments, work on the code, send it back, and after some iterations, the code is approved.
Many tools are available that auxiliaries this process. GitHub’s pull request system is one of the most commons, but others are out there.
This breaks some dependencies on the flow, creating specific steps in the whole software development flow, as well as helps to reduce the context switch for the reviewers: they can tackle reviews at their time. But of course, this comes with the penalty of possibly taking more time between the first request and final approval.
Which one should I choose?
All of them!
Each one will fit a different kind of problem, and using one should not prevent the use of another in the same situation. For example, you might have created something using pair programming but still, ask for a review from others. Or do the first session of live review and then submit the changes to a review platform.
The important thing here is to know that these options exist and use them as you need.
Why should I include Code Review in my process
Increase software’s quality
If you refer to what is identified during code review, you’ll notice that the general focus of this process is to increase the quality of the code. Reduce bugs and also preventing the code from becoming a huge problem in the future.
Increase customer’s satisfaction
No customer likes buggy software, so if the quality of the product increases, you’ll most likely see the satisfaction of the customers raising too.
Reduce cost
Going back to the story time, bugs are costly, and catching them as early as possible will reduce the cost of the product developed. This might take some time to be noticeable or even go unnoticed, but believe, it’s there.
Reduce bus factor
The bus factor is one of the terms I like the most. It is a measurement of the risk resulting from information and capabilities not being shared among team members, derived from the phrase “in case they get hit by a bus”. What would happen to your product if that one person who owns solely an important component gets hit by a bus? Would your product survive? How long would it take to get someone to take ownership of that?
Of course, we don’t want anyone getting hit by buses. But many reasons could make someone who is an important key in a project leave it. Maybe the person left the company, maybe it got send to another team, maybe it had to take a leave. It doesn’t really matter the reason, the point is your team is left with a big problem in their hands.
By implementing code reviews, even if you have someone to own by themselves something because others are also looking at the code, reviewing it, they start to get a feeling of that too. And, if a bus hit (again, metaphorically), you’ll have other people who would be more prepared to step up and assume that.
Mentor of juniors
You might’ve heard of “we learn things better by doing”, which for the dev world could not be more true. Code review is a step-up on that. Not only someone is doing, but it’s receiving feedback from people that know more about the software or the language, helping this person ramp up.
Obviously for this to work you need to count on the collaboration of the more seniors of your team, to not only correct someone but to actually explain and teach on why something should be done in a given way. In my experience, this is not a burden on the review, but instead, is an investment in the future (which pays off really nicely) and an extension of other learning initiatives your team has.
Guarantee project’s guidelines
Directly from Wikipedia, coding guidelines or coding conventions are a set of guidelines for a specific programming language that recommend programming style, practices, and methods for each aspect of a program written in that language. These conventions usually cover file organization, indentation, comments, declarations, statements, white space, naming conventions, programming practices, programming principles, programming rules of thumb, architectural best practices, etc.
These guidelines are essential for improving the maintainability of your software, and guess what process can ensure these are kept across all your codebase? Yes! Code review.
Many of these guidelines can be automated by tools and scripts, but still, human inspection is essential.
Increase collaboration
Last but not least, code review allows for increased collaboration in your team. Even if you have a developer working alone in a given task/component, because they are sending their work to review, others get to be a part of the solution too. This creates a sense of teamwork.
Tips for applying Code Review
What to start applying this or even enhancing the process you have today?
Fear not! I have some tips to help you!
Review everything
First and foremost: review everything. Just one line change? Review it. You never know when a possible typo or missing hyphen could cause problems.
Believe me when I say this: sometimes the smallest change can cause a ripple effect you cannot foresee. But if code review is stated, it can reduce the chances of some disastrous changes.
Make lists
If you are starting with reviews, make lists of what you’ll be checking. This can help you get comfortable with the flow and after a while you won’t need these lists anymore.
Here’s an example of a list you can get inspired by (source):
- Code formatting
- Architecture
- Coding best practices
- Non-functional requirements: maintainability, reusability, reliability, extensibility, security, performance, scalability, usability
Automate what’s possible
How I love automating things. And for code review, this can be your best friend. From the list I mention in the item before, you might be able to spot lots of checks that automated tools can do (instead of the human). Use them to your advantage.
It might take a few manual iterations at first (as any task you want to automate) to get a grip on what and how something should be done. But once you have this, automate whatever is possible and safe and leave the more complex steps to human reviewers.
The idea is not to substitute humans but to use machines to make our life easier.
Plan and share the load
Code review takes time and it should be included in your process and planned accordingly. When planning your and your team’s work, take into consideration the time spent on reviews. Not only on doing, but asking for reviews and iterating to fix the possible problems.
Also, share the load of reviews among every one of your team. The idea is not to make someone’s job to only do reviews. It should be a task included in the jobs description of each one, regardless of level.
Everybody gets in the game
As I mention, everybody should be included and use the review process. It’s not just seniors reviewing code from juniors. Sometimes we need the fresh eyes of someone who is less experienced to spot issues in the code. Use that to your advantage and make a rule for everybody getting into this.
Review before merge
I’m not going to lie: I already convinced myself on a few occasions that I would push/merge some code and do the review afterward, aka, after it was already in production. Such times were commonly motivated by some deadline or critical of the issue and it made sense at the time. But this is a terrible idea! And trust me, I regretted it.
If you have code in production and you need to do some adjustments, things get hard, and possibly you already lost the momentum on that task you were doing and it becomes a big source of a context switch.
So the tip here is to make such occasions as exceptions as possible. Make sure your process has a strict rule of no push/merge without review.
Time management
I already mentioned that code reviews take time. You might be looking into 1-2 hours a day for reviews if your team is big, even more depending on your position and the work you do. Deciding to take those all at once can be problematic.
Several researchers out there tried to measure the attention span of adults, but they have a hard time converging. But you know yourself: how much time can you take a task with all the attention required, not getting distracted by your phone, emails, messages on Slack, or whatever? Whatever your answer was, this should be a limit on the time you take reviews at once.
For me, this time is about 40-45min. So if I have ~2 hours of review to do, I’ll break it into chunks. Do some in the morning, as I start my day. After lunch do some more. And finally close to the end of the day. Using this tactic, I can guarantee that when reviewing, my attention is focused on that.
Embrace the subconscious implications
If you are starting to apply code review, if you are a junior, or even if you are just a regular person who puts a lot of pressure on yourself, there’s a big chance the idea of showing your code to the world, or actually to a few specific people, might frighten you. You are not alone!
I can say that until this day, after more than 8 years of experience as a software engineer, depending on the work I do or who is going to review it, I get kind of scared. Scared that people will criticize me through my work, or judge me on whatever scale they have. But I can safely say that this fear is totally a thing in my head, and I make sure to never hold me back from doing my job or ask for code reviews.
What I do instead, and leave as a tip for you, is to embrace this fear. Let it motivate you to create the very best piece of software you can. If you get some issues being raised during review, learn from them and improve yourself.
Don’t take it personally
Really connect to the last item, never take a review comment personally. You will receive critics, but they are not towards you. They are issues on a piece of code you wrote, and you can (and hopefully will) learn from those.
Final thoughts
I hope I could shed some new light for you on code review. I only scratched the surface, and there’s a lot more information on the web if you want to get deeper into it.
I really love and support code reviews. I see it as an essential part that has helped shape the developer I’m today, and even after years of experience, I keep learning new things whenever I ask for a review.
If you want to discuss something about what I talk about here, tell me your experiences with it, I’m more than happy to chat with you. As always, you can find me on Twitter at @thamy, you can send me an email at tkcandrade@gmail.com, or even come by at my LinkedIn profile.