‘The’ Lifetime Activity

Why the approach to code reviews is all wrong.

Today we perform code reviews before we check-in to the source repository.  It was not always been that way.  I can remember when code reviews were first formally introduced to the company.  The response from the team was generally skeptical.  Team members were concerned that it would slow down development, not produce measurable improvement, & act as a distraction from other high-priority activities.  I support Demings idea that “an organization does not simply need good people, but people that are improving with education”.  The team came through on this point because today we cannot imagine any team not leveraging code reviews in its day-to-day activities.

The other day I was thinking about our practices regarding code reviews and why we execute them the way we do.  After repeatedly asking myself many questions that started with ‘why’ & ‘how’, it occurred to me that perhaps our perspective on code reviews is narrow, misguided, and thus limiting new opportunities to improve the team & code base.

Allow me to explain.

There are many different types of code reviews:

There is the ‘shoulder check’ where a couple engineers informally review changes in the code and asks questions while providing feedback.  This is the most common type of review in our company.

From time to time we also have ‘formal reviews’, where a group of engineers simultaneously review code as the author walks them through it.  Typically this is reserved for large scale changes in critical areas of the code and is very time consuming.

Occasionally we participate in ‘paired programming’ where code reviews happen on-the-fly as the code is written.  This is the fastest way that I know of to remove defects as they are created.

There are other varying types of reviews, and my goal here is not to list them all, but to provide some insight on their nature.  Code reviews are formal or informal, fast or time consuming, on-the-fly or planned, executed by the individual or in groups.  It is no wonder that when people hear about code reviews for the first time that they are apprehensive.  Depending on what kind of review you practice, your experiences and opinions may be different from your team mates.

Given such variation, I think that the common premise, that code reviews are mechanisms to locate and remove defects, is inaccurate and limiting in perspective.  While the outcome is less defects and improved quality, this is merely a side effect of the true nature of code reviews:

“To learn”

Ask any engineer or manager today what the goal of code reviews is and they will likely respond “to find and remove defects”.  In reality, code reviews are much more than this.  Not only do we want the code to be improved, but we also want the participants to improve as well.  If improvement did not occur, the same type of defects would manifest again and again.  Then, as the code base grows, reviews become longer because the ratio of defects per line of code increases, and the business suffers due to the increased effort to identify and remove defects.

This is not a desirable outcome.  We want those we work with to learn and understand why a change is needed.  This growth improves not only the individual, but also the team, and eventually the company.  People who regularly learn about how to avoid certain errors, will write better code.  People who write better code can pass those learning’s on to others through other code reviews.  Eventually, the network reaches everyone, and productivity is improved.  But to do this, you must have the mindset of a learner and not a teacher.

Teaching is a natural outcome of a learning mindset.  If I approach a code review with the mindset that I will find all of the defects, then I make statements like ‘This code should be extracted in to its own method and because of this reason’.  If I have a learners mindset then I ask ‘Can you tell me about how this code works in this context?’, or ‘Are there any opportunities to refactor this code?’.  The difference is subtle, but powerful.  Instead of directing to the solution I think is the best, I engage in open-ended conversation.  This allows the author of the code to provide his or her perspective, which is different from my own.  In turn I gain insight and understanding, which may or may not, sway my initial assessment of the code.  We exchange comments and ideas, and perhaps the code is changed to remove a defect.  Perhaps my assumptions are challenged when I learn something new.  More importantly, we walk away from the review more knowledgeable than when we started.  One or both of us have learned something that we can carry in to our next code review.

Not all code reviews will be that cut-and-dry.  That isn’t my point.  I wanted to point out the contrast between a review with the aim to find defects, and one aiming to learn from each other.  You will find defects and have the author correct them, but does the author understand how not to introduce the same defect again?  How do you know the author was attempting to express an idea, one that would have taken the code down a different and better path than the one you stated?  Don’t get me wrong.  You will have successful reviews with a ‘find the defect’ mindset.  I’m saying that you and your team won’t be as successful unless you adopt a learning mindset.

Code reviews are about learning, and learning is a lifetime activity.