Have you ever came across a piece of code in your project and asked "What the heck is this?! Who even wrote this, and why?"? Or have you ever been in a situation where a bug was found in production and after locating the cause in the code, you were like "Well this is an obvious mistake. How did it manage to go unnoticed?"? Have you ever worked on a project where technical debt accumulated even if all developers were determined to write quality code? Mistakes can sneak in despite your good intentions and high level of skill.
A lot of errors in the code happen not because of lack of skill, but because they were overlooked.
Sometimes, all it needs to make a piece of code better is a second pair of eyes.
Have you ever been in a situation where someone asked you about a piece of code your teammate wrote and you answered "I have no idea. Jim (or whatever the teammate's name is) was working on that feature.". And what happens when Jim goes on vacation? No one touches the features he was working on until he comes back? What if Jim leaves the company? Someone else from the team will spend hours (or even days) getting familiar with Jim's code and trying to figure it out. Well, if you make a habit of going through each others code, situations like this will stop happening.
There are many teams in which developers familiarities of the codebase are disjunct, which can result in painful, time consuming knowledge transfers or inconsistencies inside the codebase.
Code reviews can help us avoid situations mentioned above. For code reviews to really help us, we need to find a proper way of doing them. I will give you some guidances in a moment, but first let's see some more of the benefits of code reviews.
Besides reducing chances of making unnoticed errors and creating technical debt, code reviews can also bring some improvements both to codebase and developers.
Let's say you implemented some feature. You did a good job and the feature works ok. Maybe if someone else looked at your implementation, they could notice a place for improvement or come up with idea that will make your "ok" feature great. Your job can get better by introducing a second pair of eyes.
You can also improve as a developer by making other people review your code. Sometimes it is not so easy to become aware of your coding flaws. Imagine you are a bad developer that writes bad code. It usually takes code going to production and you having to maintain and modify it, for you to realize you wrote a bad code. In some cases, when the time comes to maintain and modify the code you wrote, you might not even be the one doing it. By getting your code reviewed, you will be getting continuous feedback on your coding flaws, which will make you improve faster.
How to Do Code Reviews
So, code reviews can help us a lot. But, like with everything, there are many (good and bad) ways of doing them. To be able to benefit from code reviews, you need to establish some rules and procedures about the following:
- Who should do code reviews?
- How much code should be reviewed in a single review?
- How to submit a piece of code for a code review?
- How to give a proper feedback when reviewing someone's code?
Who Should Do Code Reviews?
In some teams there is a rule that technical team lead does code reviews to all other developers. While this approach can be useful, I don't find it really attracting for some reasons.
First of all, team leads make mistakes too, and they could also benefit from their code being reviewed by other person. Previously mentioned benefits of code reviews apply to all developers and I don't see why should team leads be excluded.
I also mentioned developers familiarity with the parts of the code they didn't write. If all the members of the team are doing code reviews for each other, they get a chance to get familiar with parts of the code they didn't write. However, if the team lead is doing code reviews to everyone, only he/she will benefit from this.
Another reason I am not a supporter of code reviews being done by the team lead only is the possibility of that person being overwhelmed by them. Imagine a team of 10 developers, each of them submitting a feature for a code review every 2 days. If only the team lead is reviewing those features, he/she will have to do 5 code reviews per day. This will take a large part of that person's working time and will definitely have an impact on his/hers output.
How much code should be reviewed in a single review?
Code reviews should not take much time. Reviewing large amounts of code at once can be exhausting and increase the chance of reviewer missing something. You should only submit one feature at a time for a code review. Don't wait for multiple features to pile up and then assign someone to review them. Code submitted for review should not be more than 500 lines long. If your feature is more than 500 LoC long, try splitting it in smaller sub-features.
How to submit a piece of code for a code review?
Many source control systems have a good support for code reviews. Pull requests are a nice way of submitting your code for a review. One possible flow could be:
- Before starting to work on a feature, create a new branch from the latest version of the development branch on your source control. Give that branch a name and a description based on the feature you are working on.
- Implement your feature.
- Publish the branch.
- Create a pull request and assign someone as a reviewer.
The person that reviews your code should provide you with a feedback about any possible mistakes or improvements. If there are any, you make the modifications according to the feedback and push them on your branch. When the reviewer approves your pull request, they should merge your branch into the development branch.
It is important that you establish flows and procedures for doing code reviews and stick to them as a team.
How to give a proper feedback when reviewing someone's code?
In programming, there are often multiple ways to do a single thing. There are cases where some approaches are proven to be better than others. In some cases, however, there are many approaches to solving some problem, but none of them is proven to be significantly better/prettier/easier/faster... than others. In those cases, opinions about which approach is better are usually divided, which can result in some endless discussions among developers as a consequence. Code review can usually be a trigger for those discussions, which can prolong the code review and make it more difficult and unpleasant. That's why you should do everything you can to prevent those discussions from taking place during a code review.
One possible way to prevent them is by establishing coding standards and guidelines. Not only coding standards and guidelines can make your codebase more consistent, but can also help with code reviews by putting the whole team on the same page. Whenever there are multiple approaches to doing some thing, none of which is proven to be better than others, choose one and add it to the coding standards/guidelines. By doing that, you will establish the "right" way of doing it.
If you are asked to review some piece of code, avoid imposing your opinions to the person whose code you are reviewing if those opinions are not conveyed by facts. As a reviewer, when you encounter a situation in which you disagree with the approach taken in the code you are reviewing, do the following:
- If your approach is proven to be better: Suggest your approach.
- If your approach is not proven to be better: Check the coding standards/guidelines for the suggested approach.
- If that case isn't covered by the coding standards or guidelines: Go on without suggesting anything. Optionally, choose the most appropriate approach for that case with the rest of your team/company and add it to the coding standards/guidelines.
Development teams can benefit a lot from doing code reviews as part of the development process. Code reviews are not only useful for more experienced developers to check the work of less experienced ones, but for many other aspects, like reducing the number of bugs, expanding the familiarity of the codebase among developers, making the codebase more consistent, help developers improve and get familiar with some new approaches, etc. However, to get the best out of code reviews you need to establish some rules and procedures about how will you do them and stick to those rules as a team.