Two Sets of Eyes, Part 2: Which of these 5 methods of code review work best?

Except in very rare cases, we don’t release code into the wild unless more than one person has looked at it. But what does “looked at it” mean?

There’s a continuum of options. If you’re the person writing the code, you can do the following:

  • Have an email conversation with another person about the code after you write it.
  • Have another person review your pull request (PR) asynchronously.
  • Have another person review your code synchronously after you write it.
  • Pair program with another person at certain points while writing the code (opportunistic pairing).
  • Pair program “full-time” with another person while writing the code.

So, which approach is best? There’s no single right answer, but we can trace some themes which will help us answer the question.

Generally, we favor a "two sets of eyes" approach, which:

  1. maximizes communication efficiency;
  2. maximizes code design quality; and
  3. minimizes rework.

You might graph them like this:

The closer you get to the email end of the continuum, the worse it is for communication efficiency and design quality, and the more likely it is that you’ll be reworking your code. The “full-time” pairing end of the continuum wins for all three dimensions. It’s not hard to see why. (Our evidence is mostly anecdotal, but it’s consistent for the last two decades.)

The first three options have something in common: They are code reviews after the fact. It’s written already, then somebody else looks at it. That often causes problems.

First, it’s extremely inefficient communication. You’re writing the code, somebody else looks at it after the fact and renders an opinion. They missed every thought you had while you wrote the code. They have no idea why you did what you did, beyond the basic syntax you chose. A reviewer can ask you why you took the path you did, but then you have to spend time explaining what you already thought about, which might be fuzzy after the fact.

Email is the worst example of this. Years ago, Ron Jeffries used to say that his innate wit and charm doesn’t come across in emails. Neither does design intent, problem analysis, nor non-verbal communication. It’s hard to imagine a worse way to make sure code is well written. Maybe smoke signals.

Second, you had zero opportunity to talk to someone else when you were designing the code in the first place. Sure, you could’ve spoken to one or more people when you started, but they weren’t with you along the way. The reviewer might give you lots of good input about design, but then you run into problem number three.

Third, you’ll very likely have to rework your code. Potentially a lot. You might even have to start over.

How effective does this sound? Not very. The first two options suffer from all of those issues. The third option (synchronous code review) fares a little better, because at least you don’t have inefficient communication with the reviewer, but it’s still after the fact, which compounds the other problems.

The last two options fare better. They’re both flavors of pair programming. We’ll spend all of the next blog post talking about that, but in a nutshell, pair programming (or pairing, for short) is two developers working on the same code on the same machine at the same time. Them’s fightin’ words in some circles, so we’ll wait until next time to dive in. For now, consider how each flavor of pairing looks for communication efficiency, design quality, and rework.

Pretty good, we’d say, most of the time.

Your communication about nearly all code matters is instantaneous. No waiting for a reviewer to share thoughts after the fact, no rework once the reviewer gets done rendering an opinion. It’s like a constant code review going on, while you’re writing the code.

Your design quality is likely to be better, too. Two heads often are better than one. One person’s brain fog doesn’t necessarily stop his pairing partner from coming up with a good idea and getting the pair unstuck. One person can challenge faulty assumptions of the other, pay attention to details the other person missed, suggest creative solutions the other person didn’t think of.

Last, but not least, you’ll probably have to rework your code far less than if one person writes it alone. If a third person reviews code that the pair wrote (or fourth, etc.), that person might see a flaw they both missed, but hey, we have to draw the line somewhere. We still have seen a third reviewer find far fewer bugs than if one person wrote the code alone.

That’s why we favor pairing over other asynchronous approaches. It’s simply better at addressing the three things we want to optimize for. Opportunistic pairing is often a good approach if a problem is relatively straightforward, and a person writing the code needs help at strategic spots. “Full-time” pairing (remember those quotes) is often better when a coding problem is particularly hairy, or big. Either one beats email reviews six ways from Tuesday.

Next time we’ll talk about what pairing is, and is not. And we’ll evaluate some of the most common objections to the practice (there’s no shortage). Make sure to leave your comments if you agree/disagree?