A lot of programmers feel like being asked to go to a code review is like being told by mom to eat our veggies. We’ll complain about it, and even if we do eventually swallow them we’re determined not to enjoy them.

It’s something I’ve seen over and over again: programmers groaning about having to go to a code review, usually because someone gets some big idea about making things better, and decides this is how you do it. There’s sometimes a little nervous joking at the beginning of the meeting about how nobody really wants to be there. And after it’s done, a lot of us get the distinct feeling that it was a waste of time.

The thing is, code reviews can be a really good thing. And not only that, they don’t have to be a chore. If you do them right, people on the team can start to appreciate them and even – heaven forbid – enjoy them.

So how do we make code reviews more palatable? We need to think about what motivates us as programmers. Programmers love to code. We love building things, and we love solving problems. But we hate anything that seems bureaucratic or tedious, and we definitely hate meetings. But most of all, we hate being in uncomfortable social situations that require us to confront people face-to-face. We’re not alone in that; most people hate situations like that.

I think that’s a pretty big part of why programmers intuitively dislike code reviews. It’s not that we’re afraid of putting our work out there for our peers to see. That’s actually something we look forward to: we love to show off code that we’ve worked so hard on, and we definitely appreciate the input from the people around us. But it’s almost never the person whose code is being reviewed who groans about it. No, it’s usually the people who are asked to attend the review. And I think I know why: it’s because we don’t like being asked to criticize the work of others, openly and without hesitation, for the good of the team. I think we naturally feel uncomfortable putting someone else in the position of having to openly confront their errors, because it’s so easy to imagine ourselves in that same position.

And that may be the secret to holding code reviews that people actually look forward to attending: make it about helping make the code better, not criticizing and finding its flaws. If you can come up with a way to avoid bitter arguments about tiny details and stubbornly-held opinions, and instead concentrate on helping someone improve his or her code, then people will stop thinking about code reviews as an uncomfortable meeting and start thing about them as a way to build better software.

That sounds like a tall order. How do you do it?

Well, you start out by thinking about one of the best things that can happen in a code review, something that I’ve seen many times. It usually happens about two thirds of the way through the review, about the time when the first signs of meeting fatigue are starting to set in. Someone points out another problem with the code, and a conversation starts up about an aspect of it that nobody really thought of. Uh-oh — it’s a bug, a particularly nasty one that. Then everyone kind of looks at each other with a weird mixture of relief and disgust, because we found a bug that a) definitely would have made it into production, and b) would have taken hours or days to track down and fix.

I’ve said this many times before: programmers are very practical people, especially when it comes to our own time. If something will save us time down the road, we’ll definitely do it. If you can show a programmer that a tool or technique (like a code review) saves more time than it costs, that’s a great way to get him or her to start thinking positively about it.

That doesn’t change the fact that a lot of people get nervous about code reviews, even people who have done them a bunch of times. So I spent a little time thinking of advice I’ve given about code reviews over the years. Some of this is pretty standard code review stuff, but I think it’s worth repeating because people have so many different views on how to do code reviews effectively. And I think that if you think about them, and get other people on your team thinking the same way, it could definitely help you hold effective code reviews.

So here’s some advice about holding better code reviews — you can think of them as code review tools (or even code review best practices, although I’m not a huge fan of that term) that can make your software better:

• First things first: there are a lot of different ways to do a code review. Some people like to follow a very strict process. Personally, I like to keep them informal; the more it seems like an everyday conversation, the more work we actually get done.

• It’s important to keep the meeting to under two hours — any more than that and meeting fatigue sets in. Most code review teams can handle between 200 and 400 lines of code in a two-hour review meeting. (Your mileage may vary.)

• Don’t forget to distribute the code before the review, and make sure you give everyone enough time to actually look through it. Send around a PDF of the code (a2ps is a good way to make it readable, and it’s got stylesheets for almost every language). Also make sure that everyone also has access to the source, and that they know how to build and run it. Sometimes it’s a lot easier to prepare for a code review if you can actually debug your way through the code.

• Make sure that everyone knows you appreciate their time. It’s easy to forget that, but it helps the team see the review as a useful tool and not just another boring meeting. Remember, you’re pulling half a dozen or more people into a room for two hours, plus preparation time — that’s the equivalent of taking a top developer off of all projects for two days. That’s also why it’s very important to choose a good block of code to review. Choose one that’s inherently risky: a difficult algorithm, code from a library that many other people depend on, an interface a lot of people will use, a particularly nasty bit of spaghetti code.

• Code reviews and refactoring work really well together. Look for opportunities to extract methods, rename variables, replace literals with constants, etc.

• Pay attention to OOP principles, especially encapsulation. Improving encapsulation is an easy and effective way to prevent bugs.

• The code review isn’t just about bringing up issues — it should also be about giving some indication of how to resolve those issues. Ideally, the programmer whose code is being reviewed should be able to read through the results of the review and very quickly implement the fixes, because in the meeting we wrote down exactly what needs to be fixed and how to fix it. (We don’t actually have to write down lines of code, of course — just enough information so it’s clear what to do.) A good way to do this is to make the goal of the meeting to be to produce a log, or a list of fixes that need to be made to the code.

• Instead of storing the log in a spreadsheet, Word document, or wiki page (I’ve done all three), try having the moderator put the results of the review directly into the code as comments (which includes an easily searchable unique string like ‘// %%TODO: CODE REVIEW 8/28/08%%’, so the programmer can find them all). The results of the review meeting can be e-mailed out a link to a diff report in the source repository. When the programmer goes back to update the code, he or she can alter the comments to make sure they make sense in context — but they can stay in the code because they’ll make more sense, and it’ll be clear why the code is the way it is if someone is maintaining it in the future.

• A good way to focus the discussion is to guide the conversation away from arguments about coding in general, and towards what to write down in the log to resolve the current issue. Make a good effort to figure out how to resolve the issue: most can be resolved in the meeting. Any issue that can’t be resolved in a reasonable amount of time gets added to the log as an open issue.

• I’ve always gotten a lot of mileage out of using a moderator. The moderator’s job is to keep track of the discussion, and keep the discussion on track. If people are getting off onto a tangent that won’t benefit the code, or if they’ve gotten into a disagreement where there are merits on both sides of the issue and it clearly won’t be resolved, the moderator should suggest that we log it as an open issue and move on. You can always follow up later and resolve the issue.

• Some people get very strict about making sure that the moderator stays at arm’s length, and doesn’t get involved with the review. Personally, I think code reviews are hard enough without imposing arbitrary rules like that (because we’re laying someone’s code bare and dissecting it, which can be difficult for anyone who’s not used to it). We’re all adults here, and we can trust any of us not to “abuse” a moderator role. If the moderator has something to say, he or she should say it. If it’s easier, replace “moderator” with “note-taker” or something like that.

• Don’t be pedantic, and try to avoid theoretical discussions. It’s really easy to get bogged down with a discussion that doesn’t go anywhere about whether this variable declaration should be here or there, whether this type of structure or that is slightly more efficient, if we could do something better in theory if we scrapped a large amount of code and rewrote it. If a discussion isn’t going to directly lead to a change, even if it’s an interesting topic, note it in the log as an open issue and move on. And definitely don’t point out spelling errors. A lot of grate programmers are lousy spelers.

• Make sure variable names make sense, but don’t worry about naming conventions. Some people love underscores in front of parameters, some people hate them. I’m sure you can come up with at least three different “official” conventions for any programming language. There are few things less useful during a code review than an argument over whether to use PascalCase or camelCase.

• One way people like to do reviews is to have people “read” the code – interpret it out loud. I’ve had some success with going around the table and having people take turns “reading” each chunk of code. If there’s a chunk that is difficult to “read”, it’s not clear enough and is a good candidate for refactoring.

• Before you distribute the code to the team, run it through a static code analyzer (like FindBugs or FxCop) and fix issues that are found. There’s no need to waste discussion time on problems that a tool can catch and log for us.

• I’ve had a lot of success with a kind of review called a “high impact inspection” (that’s a term that was developed at HP about fifteen years ago). Basically, it boils down to having everyone do the code review independently and e-mailing their issues to a moderator. The moderator puts the issues into one big list, sends them back out to everyone, and then the review meeting itself is focused on just going through those issues. Jenny and I developed a code review process similar to high impact inspections to let us hold inspections in teams outsourced to India (where time zone differences make it very difficult to regularly schedule two-hour meetings). We ran it a bunch of times, and it worked really well.

When Jenny and I were writing the section on code review techniques in our first book, Applied Software Project Management, we came up with a checklist of things that you should look for during a code review. That should give you a good starting point for coming up with a good review.

Good luck with your code reviews! If you end up with a good code review success (or failure!) story, I’d love to hear about it.