A Few Good Reasons for Code Review
After a good discussion with a colleague for the merits of code review I thought I should give some of the reasons I use as to why a development team should adopt a code review process.
Knowledge transfer
One of the big problems with long-running development projects, especially big ones, are that knowledge around certain areas tend to pile up on certain people, creating “knowledge silos”. Unfortunately, this process is also self-perpetuating, as people tend to draw to working with things they know, and avoid what they don’t know. Starting a code review process won’t even out this landscape completely, but it will give people enough of a boost to dare venture into the code by themselves, hopefully breaking any negative spiral that may have been created. It also helps bring new members of the team into the code base quickly.
Increased sense of shared code ownership
Have you ever worked with “that guy”, the one who treats the code like his own baby, and treats the developers who dare edit it as somebody who just beat up their baby? This is by far my biggest pet peeve. A code review process won’t necessarily change someone’s personality, but forcing people to let go of their code and leave it out for criticism might very well change their perception of code ownership. From the other perspective, a reviewer is well aware that they are as much responsible for the code that is submitted to the code base as whoever wrote it, hopefully increasing their sense of personal responsibility and ownership for the code, even if they didn’t write a single line of it.
More communication
Actually forcing the team to justify their code, and explain it to others opens up new channels of communication, that may well extend beyond the code review system. It invites the whole team to discuss code decisions regarding the product, or at least question them. This is very related to the increased sense of shared code.
Increased code quality
I guess this is the most obvious win that a code review system will bring. Having someone else watch over your code not only forces you to write good, understandable code, but also brings a different perspective to the problem you are solving, and may also ask questions that you never would have thought of asking yourself when you wrote the code. In fact, just having someone explain what their code does to someone else may make them realize mistakes they’ve made all by themselves. Colleagues can be excellent rubber ducks
So there you go. Four good reasons why a team should at least give code review a serious try. Do you have any reasons that I missed? Or are there arguments against code review you think are even better? Let me know!
10 Comments
Great post! Any advice on the format a Code Review should take?
We are just starting and have 3 developers who have always worked in there own silos.
Thanks
Thanks for the kind words, Patrick
In my opinion, it might not be a great idea to decide on a strict format of a code review process, given that what is to be commited to the source repository can vary a great deal (It could be just an update for the documentation, or it could be a huge refactor!).
Given that, it might be better that the team agrees on a contract that the review should fulfill, and the review format is fit to make sure the contract is fullfilled.
It’s of course up to the team what goes into the contract, as only they can fulfill it, and if they don’t accept it, it’s worthless. But if it was up to me, I think would try to fit some of the following into a code review contract
If the team is all in one place, I much prefer if the submitter can take the time to sit down with the reviewer and explain what he or she has changed, the reasoning behind it and what problems they have encountered and how they solved them.
The more actual communication goes into the review, the better. Especially if you need to tear down your silos
Great advice, Jakob (and a great post). Just to throw in an alternative to the reviewer & reviewee sitting down together…
I’ve worked on teams that did that, whereas my current team does its reviews more formally using an online tool that comes with our DVCS.
One advantage of the latter is that the reviewer has time to mull over the changes on their own – I think when your sitting down listening to someone explain what they did, it’s too easy to just agree with what they’re saying and accept the changes. The reviewer has to “think on their feet” a bit in order to spot any improvements, etc. (Another advantage is that the conversation of the code review is recorded.)
Maybe a combination of the two approaches could be appropriate…making sure the reviewer has time to prepare and come up with comments first, before sitting down with the reviewee?
Code review is important and helpful. Timing can be a problem. If I have a chunk of code that has to be delivered in a certain time-frame, how much time can I spend doing review of others’ code? With 20 workers creating code, 18 code review requests a day isn’t unusual. I ended up picking 5 people who were working in my area to spend more time on, understanding and giving feedback and just brushing by the others so I could get my own requirements done.
I agree that timing can be an issue, and I totally think it’s fine to say “not now” if you have other things to do. Code review is not usually that time-critical as long as you have time to fix eventual complaints to your submitted code.
However, I Think you are doing a disservice to your colleagues if you only skim over their code. Granted, I don’t know what (if any) tool you are using, but it seems to me that having 20 developers should mean you have 20 potential reviewers as well. Surely you could offload much of the reviewing on them?
I wish code reviews occured at my work. I have requested it time and time again, but it is seen as to be something that takes up too much valuable time. Trying to find your own bugs is far easier than with a second set of eyes. Further more, no one does their best work ‘in a vacume’. I long for the days when code review is the standard accepted practice.
It is with situations like yours in mind that I made this blog post. I don’t think a lot of people know the additional benefits of a review process without experiencing it, and there’s also a sense of fear that there is a huge time sink. However, from my experience, the review itself rarely takes more than five minutes. It should even take less than one minute if you keep your commits small. How many commits do you make per day? four? five? maybe even fifteen some days…? That’s fifteen minutes (less than smoke/coffee breaks) spent on a day to get ALL the benefits listed above.
I suggest keep trying to convert your colleagues, maybe even try to sneak something in just by asking them to look at your stuff before you commit.
Good luck, I’m rooting for you!
If you’re doing C++ development with Visual Studio, then there’s plenty of time for code reviews during the builds. I considered it a more productive use of my time than reading slashdot or these blogs – but with Visual Studio somehow I managed to do both!
Nice post, thanks. Some time ago I’ve also tried to think about many aspects of code review. I think you might find this link kaczanowscy.pl/tomek/2012-08/code-reviews-mindmap interesting.
Very interesting! I think and hope that what with agile development being more and more accepted, the code review process is also going to get more focus, and it’s important to get it right. Good to see some more people who like to think about this kind of stuff all day :)