Code Reviews Bad – Pair Programming Good (or “Don’t throw it over the wall, collaborate!”)

Why pair programming is goodI’ve had two jobs recently with clients who were not in the habit of pair programming. They did, however, have some strong procedures established for doing code reviews. Developers were not allowed to commit code to the main branch without first completing a code review. The code review consisted of creating a patch file containing all your changes, and submitting that file to other developers for review.

I learned from these two experiences (and others) how bad code reviews really are compared to pair programming.  Here are four disadvantages I have found:

1) Problems are found later, when they are more expensive to fix.

It is well documented that the later a bug is found the more expensive it is to fix.  Before submitting something for a code review, a conscientious developer will have designed a solution, coded it, written unit and integration tests for it, and probably done some manual acceptance testing as well, all in preparation for checking in.  If a problem is found in a subsequent code review, that whole process has to start again, at least to some degree.

By contrast, pair programming finds those problems earlier on in the life cycle, thereby requiring less rework to fix.  In his book on test-driven development James Grenning talks about the physics of TDD, and concludes that “If a bug lives for less than a few minutes, is it really a bug?  No, it’s a prevented bug.  TDD is defect prevention” (See “Test-Driven Development for Embedded C”, p. 6).  Pair programming is the same thing – defect prevention.  You find problems earlier, when they are cheaper to fix.

2) Context switching is required, which lowers productivity.

Code reviews don’t happen instantly.  They take time.  Other developers are busy.  What are you going to do while you are waiting for the review to complete?  Hopefully you are not going to sit around doing nothing – you are going to start on the next story.  Then the next day (or after several days, in some situations I have been in) you are going to receive feedback from the review.  In general you can respond in one of two ways: you can defend the code the way it is, or you can agree to make the suggested change.  Either way, you need to get back to the state the code was in at the time of the review.  But you have since moved on; you have made other changes to the code.

Sure, there are strategies for handling this.  You can create a patch file for your current changes and revert them.  You can create and work in a separate branch for each story.  But there is overhead in these approaches.  In addition to the context switching of the actual code, there is the mental context switching in your head.  You have moved on to other problems, but now have to get your brain wrapped around the previous issues again.  I suspect that of the two types of context switching, this one (the mental one) has the greater cost.

With pair programming, on the other hand, you are finding and addressing problems right while you are in the midst of them – no context switching required.

3) Quality is lower, because of resistance to changing working, tested code.

When asked what he thought of code reviews, I heard one developer say “It just feels like it’s too late”.  In talking further, what he was getting at was that by the time the code review happens, all this work of coding and testing has already been done.  To suggest that we tear it all up and rework it doesn’t seem all that productive at that point.  Sure, if something is really broken it will have to be addressed, but otherwise, people tend to want to leave it as is.

Another aspect of this is that even if some changes are made to fix a problem, they often tend to be band-aid solutions, because you are now retrofitting existing code; the end result won’t be of the same quality as a pair working together from the beginning.  It’s like the difference between building a house, then having an inspection, and then fixing the problems the inspector found, versus building a house with an inspector alongside you the entire time.  No band-aids placed over things, just quality code from the ground up.

4) Bad feelings can arise between team members.

This one is less common than the others, but I have seen it happen.  While we would like to think that we are  all objective-minded and don’t become emotionally invested in our code, that isn’t always the case.  And after designing, coding, and testing it we are more invested in it than if it was still just in the idea phase.  In a code review, it is our working, tested, finished product that is being critiqued.  When pairing, there is still a lot of critiquing going on (probably even more than in a code review), but it is happening earlier in the process; usually in the idea phase when we are less invested in a specific solution.  Overall, code reviews can feel more adversarial than pair programming (which feels very collaborative).

It’s all about collaboration – don’t just write something and throw it over the wall for a code review. Pair program!