Monday, September 02, 2013

code review: runtime errors only

I've continued to think about code review and how it should fit into the software development process. By applying concepts like value stream and one piece flow we can determine what the useful outputs of code review are. From there we can determine how to better focus code reviews, and even if we should be spending the time and effort to have code review at all.

Probably the first question is if we should even be doing code review. Would we be better off to just skip code review and thus be able to move faster in development? I still believe that done right there is positive return in the time and effort spent on code review.

The value is in identifying and fixing runtime errors. As is well documented the cost of dealing with defects rises exponentially the further along the code is before the error is found. With code review we can still fix defects very quickly and inexpensively. The code isn't in a build yet and it hasn't gone to any testers or users outside development. This is probably the last point where real errors can be quickly fixed with a minimum of cost and drama.

That is the entire value of code review. Anything else is waste. I've seen perfectly good code that works rejected and sent back for rework with bogus subjective "defects" that are just noise. If it's not a runtime error then it's not a defect.

Code review can be guided by these two simple bullet points. There are two purposes to code review

  • identify runtime errors
  • move the unit of work along as quickly as possible
the cost of sending code back for rework is very high. each iteration requires more design, more dev testing, another iteration of code review. and the time spent on the original rejected material is partially or fully wasted. additionally there is a corrosive hidden cost that if a developer has to revisit something then of course he is pulled away from some other value-add activity which could advance the product

thus rejecting code and sending back for rework should be avoided. it should only be done for runtime errors, where the cost of a code review iteration is much less than if the error is exposed at a later stage.


I know that some believe that code review is an opportunity for coaching or teaching. This is misguided. On the teams I've spent my career on everyone is always experienced. 2, 5, 10, 15+ years of software development experience. at this point if the developer hasn't figured out or isn't motivated to write good, clean, fast, maintainable code then he's just never going to. and there's nothing code review can do about it.

maybe if you're dealing with interns, entry level people in their first year, or people working with some new to them technology that's about the only time there's use in pointing out a better (in your subjective opinion) way to do something.

No comments: