DaedTech

Stories about Software

By

How Code Review Saves You Time

Editorial note: I originally wrote this post for the SmartBear blog.  Check out the original here, at their site.  While you’re there, take a look around at their offering.

Physical labor is one of the most strangely enduring mental models for knowledge work.  Directors and managers of software development the world over reactively employ it when nudged out of their comfort zones, for instance.  “What do you mean ‘pair programming’ — we’ll get half of the work done for the same payout in salary!”  And that’d be a reasonable argument if the value of software were measured in “characters typed per minute.”

WorkHarder

Most of the skepticism of activities like unit testing and code review originates from this same “knowledge work as labor” confusion.  The core value of software resides in the verbatim contents of the source code files, so stuffing all of the features in them ahead of the deadline is critical.  Testing and reviewing are “nice-to-haves”, time and budget permitting.

The classic response from people arguing for these practices is thus one of worth.  It goes something like this: “sure, you can cut those activities, but your quality will dip and there will be more escaped defects.”  In other words, these ‘extra’ activities pay for themselves by making your outfit look less amateurish.  That argument often works, but not always.  Some decision-makers, backs truly to the wall, say “I don’t care about quality — my job depends on shipping on June 19th, and we’re GOING to ship on June 19th, whether it’s a finished product or whether it’s a bag of broken kazoos and cyanide with a bow on it.”

I’d like to take a different tack today and fight a time argument with time arguments.  Instead of “sure, code reviews take extra time but they’re worth it,” I’d like to explore ways that they actually save time.  Whether they save more time than take is going to vary widely by situation, so please don’t mistake my intent; I’m not looking to argue that they’ll save you net time, but rather that they are not exclusively an investment of time.

Others’ Time

If your organization is largely silo-ed, this might not immediately occur to you, but the organization pays salary to folks other than the software developers, including testers, operations people, helpdesk, field support, etc.  And because it pays these salaries, the organization cares how those people spend their time.

If you code up a defect that escapes into production, then yes, you (or someone in your group) are going to spend time fixing it.  And, yes, it’ll probably take you longer to fix the later you catch it.  But it will also take a lot more than just you or one of your cohorts to fix it.  There’s the helpdesk tech that fields the call and spends time with the customer trying to reproduce it.  There are the tier 2 and 3 people to whom the issues is escalated, and the operations folks with whom they work to see if the problem can be addressed with a configuration management approach.  On the other side of it, assuming code changes are necessary, there is the tester that has to validate the fix in each environment as it’s promoted.

Contrast this with what happens if a code review catches this bug.  The reduction in the dev team’s time to fix is a drop in the bucket compared to the holistic savings from all of the people I’ve mentioned not having to do any work on this at all.

Bob’s Vacation

I hear a lot of companies talk about “bus factor,” which is a macabre visual for the degree to which team members have special, bottleneck-creating knowledge.  “Oh, that’s Bob’s section of the code, and he’s the only one that should touch it” is a classic symptom of a group with a high bus factor.  Code review dramatically mitigates this by promoting collective code ownership.

But forget for a moment about “bus factor” and let’s consider the more benign “vacation factor.”  If Bob is hit by a bus, everyone knows what to do the next day — they immediately start everyone on a crash course in Bob’s code.  But if Bob goes to Bermuda for 2 weeks, not so much.

I’ve seen heavily silo-ed groups completely alter project schedules because Bob has a vacation scheduled.  “Oh, there’s no way the new feature set goes out in August as planned because, remember, Bob always takes his vacation in late July.”  The business loses 2 weeks of calendar time to Bob’s specialized knowledge.  In a world with routine code review, things don’t go this way, and you get those two weeks back.

Onboarding

Collective code ownership helps not only when people go on vacation, but when new people start with the team as well.  A team in the regular habit of sharing knowledge will be able to bring a newbie up to speed much, much faster.

I’ve seen shops where tenured developers were overworked to the point of stress, but where newbies would sit around for literally months, twiddling their thumbs and trying to figure out how to be useful.  I consider that to be a completely unacceptable and ludicrous state of affairs, and yet, it’s surprisingly common.  Code review won’t eliminate the time needed to grasp a problem domain, but it does force interaction between newbies and experienced folks, which significantly reduces onboarding time.

Bad Habit Prevention

A final time-saver that I’ll mention is the subtle one of bad habit prevention.  I can’t count the number of times I’ve seen a programmer develop and repeat a habit for months, only to learn later, in passing, that the habit is problematic.  Perhaps it’s something that prevents all of their code from being used in multi-threaded contexts or something that invites a memory leak.

Once this is revealed to the now-crestfallen developer, he has two choices: leave the problematic code in place, hoping for the best, or undertake a massive effort to go back and change all such code from the last number of months.  Most shops, sooner or later, wind up going the latter route.  What an utter waste of time — an early code review could have prevented the rework and stopped the developer from acquiring a bad habit that might manifest itself from time to time, even going forward.

Time Savings is Only Part of the Whole

If you adopt or expand your code review practice, you will save time in the ways I’ve outlined here.  Will it be more time than you spend doing the reviews?  I honestly have no idea.

But also recall that it doesn’t entirely matter.  The standard arguments about code review improving quality, enhancing collaboration, spreading knowledge, etc. still apply.  Code review has a lot of benefits besides simple time savings.  But it’s also going to save you some time here and there in ways you hadn’t previously considered.

6 Comments
Oldest
Newest Most Voted
Inline Feedbacks
View all comments
Kevin Mote
7 years ago

Great points, thanks! (Small nitpick: the situation with Bob is an example of “Low” bus factor, not “High”. High is good, low is bad.)

Erik Dietrich
7 years ago
Reply to  Kevin Mote

Interesting. I think I’d always just assumed, intuitively, that “high” would correspond with it being significant (thus “high bus factor” would mean that the proverbial bus would cause a lot of damage). Looks like I should have confirmed this understanding instead of assuming it 🙂

Kevin Mote
7 years ago
Reply to  Erik Dietrich

Yeah, I make the same mistake with “learning curve”. I always want to say that a “steep” learning curve implies that something is difficult to learn, whereas it actually corresponds to how quickly something can be learned. So a steep curve means something is easily accessible. It’s non-intuitive, but most people know what you mean either way, so it doesn’t matter much I suppose.

Meindert Hoving
Meindert Hoving
7 years ago
Reply to  Kevin Mote

Steep learning curve is still sort of correct, C++ isn’t that hard after you climbed the ‘hill’ but if you don’t climb it you can’t do anything with it. So steep equates the amount of learning you need before you can use it. Here is a quote from wikipedia: Arguably, the common English use is due to metaphorical interpretation of the curve as a hill to climb. (A steeper hill is initially hard, while a gentle slope is less strainful, though sometimes rather tedious. Accordingly, the shape of the curve (hill) may not indicate the total amount of work required.… Read more »

Daniel Aston
Daniel Aston
7 years ago

I quite like the idea of silo-ed code, because it helps keep a clean codebase. The problem with collective code ownership is that not all developers can know the design intent of each module. They can make a change that works and keeps the tests passing, but their changes may still go against the design intent. Real example : a perfectly competent developer tweaks some screen-tabbing code. It works as intended. But he didn’t know the existence of classes dedicated to different tabbing strategies, and made his changes elsewhere. That’s one step toward code degradation. A few more changes like… Read more »

Erik Dietrich
7 years ago
Reply to  Daniel Aston

It sounds like the driving premise here (that I agree with) is “having someone other than the original author touch code is riskier than having the original author do it.” But where we diverge, I think, is how to mitigate that risk. “Have the original author always do it” works, but only under specific circumstances. What do you recommend if the original author goes on vacation, gets sick, goes on leave, transfers departments, quits, etc? Heavy collaboration in the code base follows the general mantra of, “if it hurts, do it more, until it no longer hurts.” Either way, there’s… Read more »