Expert Software Development, Staffing & Consulting

In the series opener, I wrote about “optional” software development practices like Test-Driven Development (TDD), Continuous Integration (CI), data migrations, one-click deployment and code review which are fundamental to how we build software at Fairway, but are often touted as optional and embraced by a small percentage of development shops.

In this post, let’s dig into one of the “optional” heavy-hitters, code review.

There is any number of reasons why code isn’t reviewed at all shops – every reason from the presumed time/cost commitment being too high, to just not understanding the true benefit or how to get started, to a general aversion to change. My two favorite reasons include protecting developer morale (i.e. developers can’t handle having their work critiqued) and naively thinking there’s no need/value since developers are producing a good product already. Oh brother. Perhaps worse than avoidance, though, are the truly unfortunate cases where the fears of the skeptics are the reality, where teams preform code reviews poorly.  Here, a lively roomful of peers nitpick and bash away on Johnny’s code or, quite the opposite, the review becomes a total snooze session. In either case, the review is probably scheduled too late in the project lifecycle so there’s no time for anything constructive to be done in response.  There’s absolutely no value – just wasted time.

Fortunately, code reviews don’t have to be run that way and they absolutely shouldn’t be avoided. There is real value in every code review. Whether it’s the coder, the reviewer, the project or the customer, something will undoubtedly improve with the simple act of a second set of eyes reading a few blocks of somebody else’s code.

Fairway advocates ad hoc reviews which are focused on a current coding task.  We prefer informal, one-on-one code reviews which are initiated by the coder rather than big-meeting, mandatory reviews which are often scheduled by the powers that be. We believe a responsible coder is best fit to decide WHAT is worth reviewing, WHEN it is worth reviewing and WHO would be best to perform the review.  The bottom line is we put the onus on the developer to get their code reviewed and how they go about it. With the coder calling the shots, reviews are more likely to happen often, throughout the project lifecycle.  Reviews become comfortable, effective, and efficient and guaranteed to be worthwhile.

Let’s get into the WHAT, WHEN and WHO, but first it’s very important that every team member recognize WHY.  Again, there is value in every code review.  The primary benefit, of course, is to have cleaner, tighter and easier to understand code as the result of any code review.

Code quality alone should be enough of a reason for your team to get started initiating reviews, but there are additional reasons to be kept in mind:

• Cross-training – Developers are often “siloed”, assuming sole ownership over designated areas of the application. Though other developers might reference their code, it’s often a “black box.” Code reviews open the code to team members and offers insight into complete solution development and support.
• Learning – When it comes to pair programming, there’s often the question of how to best pair developers.  Do you pair juniors with juniors, seniors with seniors or strive to pair folks with mixed experience?  It doesn’t really matter – every coder has something to offer whether it is a keyboard shortcut, fancy LINQ syntax or even the worst code which just makes us think.  With every code review someone will walk away wiser or at least with a different perspective.
• Presentation / Communication – The last time I checked, Americans fear public speaking over anything else – even death.  Couple that with the fact that most developers are introverted, well, it’s a wonder that developer-driven demos ever, ever happen.  But the ability to present, never mind communicate, is such an important developer skill and walking a fellow coder through one’s code is an easy way to start building/polishing those skills.
• Team work – Sharing one’s code forces team collaboration. Once the benefits of code reviews are discovered, it will become second nature and your team won’t be able to get along without it. Perhaps this will instigate a desire to start pair programming and, if you’re really, really lucky, the team will start to gel.  (Gel is a PeopleSoft reference – possibly the greatest soft skill, software book of all time.) And, according to that How to Win Friends & Influence People book, it’s a common misconception that you should offer help to those you wish to like you.  Instead you ask them to help you – with a code review.
• Project – Though developers will undoubtedly benefit from code reviews, the project is the biggest benefactor having a solid, understandable, maintainable code base, and a well-oiled team in place.

How does a coder know WHAT is worth reviewing?

First and foremost, we highly recommend a review of critical, non-trivial code.  After that, it’s really up to the developer to make an honest assessment of their work and request a review when they deem fit.  But everyone should agree that coders aren’t exactly impartial when it comes to judging their own code.  In other words, “elegant” to one developer might be considered “overly complex” and/or “difficult to interpret” to others.  So the key is to be honest and considerate. We suggest developers think about those who will use/maintain their code – that often helps draw the line between reviewable and non-reviewable code.

As for WHEN and HOW, here’s our suggested, ad hoc code review process in a nutshell:

• The developer hits to a logical stopping point in a critical, non-trivial coding task or any code they deem reviewable.
• The developer calls a reviewer to their desk and holds the review.  A review should last less than 15 minutes in most cases. The reviewer’s time should be respected and appreciated.
• The developer provides an overview of what the code should do.
• The developer calls out any areas of concern or areas they are particularly interested in improving.
• The developer walks through each line of the code.
• The reviewer should read the code and listen to the developer’s explanation.
• The reviewer should keep the review moving but should ask questions or offer recommendations at any time.
• The conversation should be open and safe. One of the easiest ways to accomplish this is for the reviewer to share praise along with their refactoring suggestions.

That’s all there is to it!  Now it’s time to start doing it. The next time you are about to commit your code, ask yourself if your code (or your fellow developer, or the project, or the customer) would benefit from a second pair of eyes.