Sunday, 28 May 2006
Why I Prefer Pair Programming to Code Reviews
After a few months of experience with traditional code reviews at my
new employer, I don't see any reason to recommend them over pair
programming. They create a great deal of inefficiency with very
little benefit.
The problem with traditional code reviews is that they come too late.
The best time to get feedback is as soon as you make a mistake, not
two hours later (or four hours, or the next day). By the time a
reviewer sees a change, the code is fairly well polished and most
issues have already been fixed. When things are running well, there's
not a lot the reviewer can do to add value. When they're not going
well, backtracking is expensive.
The code review process inherently encourages perfunctory reviews.
The most common review I get is lgtm, which stands for "looks
good to me". One bit of information isn't real feedback, and yet it's
what I want at that point. Don't slow me down, please.
Trying to be a good citizen when it comes to code reviews can easily
make you into the bad guy. When someone is about to check in, the
code they've written is good enough by their standards, or they
wouldn't be checking it in. The last thing they want is for someone
to tell them "yes, but there's a better way to do it, which involves
completely rewriting your code." I don't enjoy having to make the
tradeoff between quality and slowing people down and getting on their
bad side, and it's ridiculous to have to make this tradeoff when
there's a better way.
(Of course, you can say "it's good enough to check in, but here are
some things to fix." But most of your suggestions will go on the
to-do list to be fixed later, if at all, because by then, the
developer needs to move on to the next thing.)
The code review process is bad for concentration, both for the
developer and the reviewer. Because there's usually a delay waiting
for code review, many developers work on multiple things at once and
switch between them at submit time. As a reviewer, you need to check
your email reasonably often, and spend time understanding code other
than what you're working on. (Not to mention being distracted by
other email that you could have put off.) All this context-switching
is expensive. People naturally try to avoid context switches by
submitting bigger changes than they would otherwise. A reviewer's
natural response to receiving a big change is to to put if off until
they can devote more attention to it, resulting in higher latency, or
to skim, resulting in less thorough reviews.
(One way to increase efficiency in the face of this latency is to add
a buffer. Submit a series of small patches, each of which builds on
the last, and just keep going while waiting for code review.
Unfortunately, conventional source control systems don't support this
very well, and even the best pipeline wouldn't get rid of the cost of
rolling back and doing something else when the reviewer finds a
serious problem.)
By contrast, as I've written
before, pair programming gets rid of all these sources of
inefficiency. You always talk with your reviewer before coding, which
gets rid of lots of backtracking. The granularity of reviews is the
smallest possible - you can tell people about typos as they type.
Code review while pairing causes no interrupts because the reviewer is
already devoted to the task. You get much more detailed review for
the same reason. Nobody feels bad about fixing nitpicky stylistic
issues right away, and since you and your reviewer are on the same
side, you have the same perspective about what's important and what
can be put off until later. Teaching and learning are more efficient;
you can ask your partner a question without wasting time in a five
minute explanation of what you're working on first. Furthermore, as a
reviewer, you not only see the code, but your partner's whole way of
working, so you can point out ways they can do it faster, such as
IntelliJ shortcuts or fixing their build process.
Yes, there is a cost to having two people working on the same task.
In my experience, that cost is far more than paid for from all the
efficiency gains. I just hope I can convince a few people to work
with me before I lose all my good habits.
respond | link |
/code
|