[​​Brown Belt] Your code may not be peer reviewed on day one, but expect it within the first several months.

Many programmers loathe, detest, and double-plus unlike code reviews, but there’s really no reason to hate them. In fact, experienced programmers look forward to code reviews​—we’ll see why shortly.

Person and Perspective

The reason so many code reviews go bad is because the programmer makes a connection between their code’s worth and their self-worth. When reviewers point out problems in the code, the programmer takes it as an insult and gets defensive, and things go downhill quickly.

Let’s be very clear about this: reviewers will find fault in your code. Gonna happen. I guarantee it. That does not mean you suck. There’s always room for improvement, or at least different perspectives, on how your code should be written. Treat the code review as an open discussion, not a trial where you’re the defendant.

“Faults” can range from bugs to issues of style. The bugs are easy; you have to fix them. Everyone, novice and expert alike, screws up now and then, so nobody in the room thinks you’re an idiot. Just take a note and move on.

The more contentious problems arise with issues of style. Maybe you’re using a loop with a counter and a senior programmer reviewing your work suggests using an iterator instead. Is your code wrong and the suggestion right? No, matters of style are not so absolute.

Since this is an open discussion, go ahead and discuss the merits of the suggestion. Perhaps the reviewer will say, “Using an iterator eliminates the possibility of an off-by-one problem.” Don’t get defensive and argue “But my loop doesn’t have an off-by-one problem!” The reviewer knows that already. The point he’s trying to make is, it’s good style to eliminate that possibility with a different approach.

Once you understand the point the reviewer is making, thank him for the suggestion, take a note, and move on. Consider your course of action after you’ve had time to let the tension of the code review dissipate. Disagreements on style become contentious because they become personal; if you consider the merits of the disagreement without the other person right there, you may discover the reviewer had a valid point.

Perspective is essential: it’s not about you being right and the reviewer being wrong. It’s about good code and better code.

Formats

I’ll describe the formats I’ve seen for code reviews and give you some tips for each.

The Ad Hoc Review

Often you’re puzzled by something, and you just need someone to help you through it. Or perhaps you’ve found what you think is a good solution, but you’re not sure. Go grab a more experienced programmer. Even the grumpy ones usually put their grumpiness on hold; the flattery of being asked for their opinion softens even the most surly.

Buddy System

Some projects will require a “buddy” to sign off on any code that’s checked into the source repository. You’ve made a change, tested it, and now you need someone to review it before check-in. Do not just go find your favorite pal who will green-light anything you write. Go find someone who is an expert in the area you’re changing. Failing that, find someone who hasn’t buddied for you in a while.

Use the buddy system as a way to get more people familiar with your work. Especially when you’re the new person, there’s no better way to build your credibility than with code. It doesn’t have to be brilliant, wicked, fancy code—just solid code. Make sure people see it on a regular basis.

The High-Level Review

This is often a sit-down meeting with multiple people and a projector. You’re often reviewing weeks of work but at a high level. You explain the design, explain how it translates to code, and then review key portions of code. This is an opportunity for discussions on design and style. Be prepared for criticism, and keep in mind the issues I discussed about people and perspective.

My favorite question, as a reviewer, is “Let me see the tests.” I require automated tests for any project I lead, so if the response to this question is a blank stare, the review is over, and we’ll schedule a new one for the next week. However, the experienced programmer will start the review by going over the tests. Nothing instills confidence in your code better than showing the tests.

The Line-by-Line Review

The most tedious, soul-crushing code review is where everyone walks through the code line by line. In practice, this kind of review tends to be held for code that’s already a disaster. (Better not be your code.) Given that you’re in bug-hunting mode, ask of each line of code: what are the assumptions of this line? What ways could it fail? What happens in the failure case?

How do you avoid the line-by-line review of your code? Easy: get your code reviewed early and often. Use ad hoc or the buddy system, or schedule your own group reviews. I started by saying that experienced programmers look forward to code reviews. That’s because they prefer to get feedback early and avoid getting into the mess that requires the line-by-line review.

The Audit

I’ve heard about this practice from others but not used it myself. In an audit, a senior programmer takes your entire project and drills down on specific topics. And when I say drill down, I mean way down. Why did you choose such-and-such design? What data did you have to prove your assumptions? How do you prove (or test) that your implementation is correct? How much wood could it chuck per second if it could chuck wood? You get the idea.

Preparing for an audit is a big deal because you don’t know what the auditor is going to ask. You have to be prepared for anything. My only advice here is to ask yourself the same kinds of questions as you program. If your program is reading data from a file, ask yourself, what assumptions am I making of that file’s format? How am I testing those assumptions? How big can the file be? Can I prove that?

Of course, you can follow the rabbit hole down only so far. At some point there’s a diminishing return on this line of thinking; that point will vary depending on the type of project and the phase of its life cycle. Err on the side of caution with production code. Err on the side of git-er-done with trade show demos or proofs of concept.

Code Review Policies

Policies surrounding code reviews range from nonexistent to institutional. If the development style of the team is on the chaotic end of the spectrum, they probably don’t do code reviews unless absolutely necessary. That’s when you have the line-by-line review that saps your will to live. If the team uses a development practice like Extreme Programming, code review is constant: XP’s pair programming, in effect, reviews the code as it’s written.

Some industries require code review for certification purposes. If you’re writing software for avionics or nuclear power plants, there’s a grueling review process before you can ship. You know how most software comes with an end user license agreement that basically says the software has no warranty and may blow up at any moment? In avionics, there’s no such easy way out—people’s lives really do depend on your code.

For the rest of us, however, there’s no One True Way to do reviews. The best policy reviews code when it’ll do the most good, and that varies based on the team and the project. (Tip: the answer is never “never.”) An experienced manager or technical lead should set policy as needed.

Regardless, you can always call a review. When you want extra eyes on your code, ask without shame. Experienced programmers do it all the time. When a junior programmer doesn’t ask for reviews, that’s a certain sign of trouble brewing.

Actions

Take the initiative for your next code review: ask someone to buddy the next set of changes you want to check into your team’s code base. But before you grab a buddy, do a little homework:

1. Generate the list of files you’ve changed. The source control system should tell you this readily; it’s usually the status command.

2. Pull up diffs for each file, preferably in a graphical diff tool that lets you see both the original copy and your copy with the changes highlighted.

3. Now, don’t skip this step, look through the changes yourself, and make sure you can explain every one of them. I’m not talking about an audit-style drill-down on your motivation for every line of code, but look for obvious goofs. There’s also a good chance you left some cruft in there on accident; fix that and pull new diffs.

Now grab a buddy. If your team doesn’t do this by policy, just ask another programmer—preferably someone senior to you—something like, “Would you mind looking over my changes before I check them in?”

With your buddy nearby and the diffs on-screen, explain the objective for your changes, and then walk through the diffs for each file. You can drive or the buddy can, whatever works best. Assuming you program with decent style (and why wouldn’t you?), the buddy should be able to scan your changes quickly.

In addition to explaining the code, explain how you tested it. Ideally you have an automated unit test suite; review this code too. If not, explain any testing you did by hand or any reasoning about the correctness of your code.

Chances are you’ll catch a couple goofs before you even call your buddy over. Plus, your buddy will ask a question or two you hadn’t thought of. You may find that you want a check-in buddy for every commit.