The first experience that I had with code reviews was a bad one. I was working on a project and one of the people who had been there a while, gave me an ominous warning about the horror of their code review process. He said it was so harrowing that developers often called-in sick on code review day, to avoid the trauma. They were going to rip me a new one. Yeah, that guy was just a breath of sunshine. Nice.
The whole thing struck me as being really odd. Why would a company sponsor a hazing ritual like this? If people were calling-in sick, to avoid it, then what was really gained, other-than, cultivating an environment of fear and tension. It sounded pretty insane.
Later, I got to hear about the good things that come from a code review process:
- A senior programmer reads another person’s code and makes recommendations
- Other programmers get to observe the code and learn about the things that were good and the things that were bad
- A programmer might recognize a block of (somebody else’s) code that can be shared or consolidated
- It cultivates an environment that encourages quality
- The whole team gets involved in the definition and enforcement of the process, standards and conventions
- It makes people think more deeply about what they are doing
- It keeps people accountable for adhering to standards/conventions
- For junior programmers, it is a form of training that is focused precisely where they are weak (instead of a taking a 40 hour course where only 1.5 hours of it is new material)
- It stimulates conversations between teammates
When you really think about these items, you can see that code reviews have the potential to produce some gigantic ROI. For example: if you have a few junior developers (recent college grads), you can get your top developer to review their code and make recommendations, focused at improving the weak-points. The junior developers (“cheap staff”) are rapidly learning to program like pros (“expensive staff”). All of that high-quality code gets written at a fraction of the price. That is big ROI for a company and excellent growth potential for your staff.
Not just for n00bz
You might think that code reviews will only benefit junior programmers, but senior programmers benefit as well:
- If they are in a hurry or under pressure, they might cut a few corners sometimes. They can usually justify it to themselves but not to others.
- The top dog(s) are rarely questioned.
- If questioned, it is easy to make an assumption that they know best and not double-check that assumption.
- It is possible for, even an experienced person, to be misinformed about something or misunderstand something.
- Sometimes, senior developers might not be aware of new techniques or technologies or assume that the new stuff is inferior to “tried & true” stuff. (the devil you know vs. the one you don’t)
Every member of a development team needs to be challenged and new technologies should be assessed by the entire team. No assumptions should ever be accepted at a code review. You prove your point-of-view to the team, so they can see how to develop correct conclusions (or to correct invalid assumptions). Win/win.
Psychology of it
So, you can see how this process can be valuable, but you can also see how it can be quite humbling. If you are “Billy Bad-Ascii” and you think your code is epic, but somebody finds any kind of fault in it, you might feel a little like Goliath. People with a well-developed ego are going to feel very vulnerable. Things could get personal with little or no effort.
If the code review process turns ugly, then your ROI will drop rapidly or get completely lost. People aren’t going to learn much if they feel defensive or vengeful. Certainly, they will learn nothing if they call-in sick, to avoid it.
This reminds me of the times, in the US Army, when we went to “the range”. This is not your normal environment. Things could get serious in a hurry. So, before you hand-out pistols and ammo, you better start-off with some safety rules. No horsing around while you are on the range. Judicious marksmanship and impeccable manners are mandatory! No exceptions!
Rules for safety
Here are the rules that you should read at the beginning of EVERY code review. If you want to get old-school, you could even have the developers stand up and recite these rules out-loud, before you begin:
- We are reviewing the code, not an individual.
- We are trying to learn and improve ourselves. Remain focused on this goal.
- Check your ego at the door. Open your minds.
- We are professional programmers. We are striving for excellence.
- We make no assumptions. If asked, we prove anything/everything. Be prepared to do so without hesitation.
- These people are our co-workers and our friends. Tact is required. Mind your manners.
- The company is paying us to do this. We are obliged to show a good ROI.
You also need to introduce a special code review assertion technique: “calling a foul”. Just like a football referee, if somebody is demonstrating “unsportsman-like conduct” anybody can throw a flag on the play, halt things and require corrective action (an apology, or equiv). You will find that it curbs poor manners quickly and encourages people to think before they speak. (effectivly keeping emotions out of the process). After a few weeks, you won’t need it any more.
To maximize the value in a code review, here is the timeline that you should follow:
- 1 week prior, the code review should be scheduled and the code-to-be-reviewed should be identified. It should be 400-1000 lines long with a target time of 35-45 minutes
- 2 days prior, the code should be copy/pasted into a MS Word document and reviewed for 1 to 1.5 hours.
- Highlight interesting code
- Annotate observations in a bright color, like burgundy.
- Code-reviews should happen at 11 am. After the code review, people might feel inclined/inspired to have side-discussions. Having such discussions over lunch, in a more relaxed environment, is ideal and should be encouraged.
- During the code review:
- Start with 10 minutes to go over the rules.
- Review for 40 minutes.
- Take 10 minutes to wrap-up, record observations and schedule any follow-ups.
- After the code review, any necessary corrections/improvements should be applied and the reviewer should follow-up to ensure that this is done.
- If any changes to the team process, standards, conventions are required, then those should be assigned and the reviewer will follow-up to ensure that it is done.
The frequency of code reviews
When a new project has started or when a new team-member is added, code reviews should be done weekly. Once the code reviews become boring or mundane, they should be scheduled monthly. Always try to review some new code that might be interesting. Try not to focus solely on one person.
Why doesn’t everybody do code reviews?
The biggest risk to the code review process is the team itself. If you ask any company/team [that doesn’t do code reviews], “why not?” they will tell you that they don’t have the time. When you think about the ROI, it doesn’t make sense. Is there really no time, or is it something else?
The reality is this: Preparing for the code review itself does take time. Your top developer is going to feel pressure to complete programs and work on his normal work tasks. Those things are measurable and accounted-for. In contrast, code reviews are abstract and the benefits are ubiquitous. Committing to any process (like this), is going to consume time. Combine that with the deterrents (ego threats) that go along with code reviews, and you have a formula for awkwardness. All you have to do is [not water that house plant]. It will wither and die. (Then, put it in the shed and forget about it.)
One approach that I have seen, that works, is to rent a reviewer. Somebody from outside your company (who is getting paid hourly) is going to have a higher level of determination to stick to the schedule (or else, he doesn’t get paid). He will be your work-out buddy. He will help you stick to the schedule and make sure you get a good value from your code reviews (from fear of getting cut). Think about it. This has to be one of the tightest values you could ever get from a consultant. Plus, you can have the consultant give an objective evaluation/summary report to the manager/boss. Bonus-win for the manager!
So, if you want to get continuous, ongoing value from code reviews: follow these steps, stick to the rules (of ettiquete), and find a good consultant and maximize your ROI.