16

We've introduced a code review process within our organisation and it seems to be working well. However, I would like to be able to measure the effectiveness of the process over time, i.e. are we not finding bugs because the code is clean or are people just not picking up on bugs?

Currently, we don't have an effective fully-automated test process. We primarily employ manual testing, so we can't rely on defects found at this stage to ensure that the code review process is working.

Has anyone come across this issue before or has any thoughts on what works well in measuring code reviews?

6
  • 8
    Finding bugs is not the only purpose of code reviews. They are also useful for reinforcing coding standards, cross-training, cross-pollinating of ideas and technologies, etc
    – Jason
    Jan 12, 2012 at 16:59
  • Thanks Jason & understood, however in this case I'm trying to figure out how to ensure that the process achieves its core aim of defect prevention as early in the dev process as possible
    – Johnv2020
    Jan 12, 2012 at 17:05
  • @Johnv2020 That is not its core aim though... You completely miss the point of a code review. This would be like putting in a great new safety feature on a fleet of jet aircraft, then trying to judge its effectiveness based on the number of crashes. There are too many variables and other factors to consider to accurately make any claim that the safety feature improved the odds of a crash not occurring.
    – maple_shaft
    Jan 12, 2012 at 17:45
  • @maple_shaft: Weak analogy. Trying to gauge bug rates is more like trying to measure the number of coffins used for dead people from crashes.
    – S.Lott
    Jan 12, 2012 at 17:52
  • 1
    In all the code reviews I've attended, many bugs have been fixed already in unit and higher-level testing. That is, code is not ready for review just because it compiles. Jan 12, 2012 at 20:52

2 Answers 2

8

There are a number of metrics that can be gathered from code reviews, some even extending throughout the lifecycle of the project.

The first metric that I would recommend gathering is defect removal effectiveness (DRE). For every defect, you identify what phase the defect was introduced in and what phase it was removed in. The various defect detection techniques that you use are all assessed simultaneously, so it applies equally to requirements reviews, design reviews, code reviews, unit tests, and so on. You would be particularly interested in the number of defects caught in the code phase, since this would probably encompass your unit tests as well as code reviews. If many defects from the code phase are making it through to the integration test phase or even the field, you know that you post-coding practices should be evaluated.

Various meeting metrics would also be relevant. These include the time to prepare, time in meeting, lines of code read, defects found in the review, and so on. Some observations can be made from this data. As an example would be if your reviewers are spending a large amount of time reading the code in preparation for the review, but finding very few problems. Coupled with the DRE data, you can draw the conclusion that if defects are being tested in integration testing or the field, then your team needs to focus on their review techniques to be able to find problems. Another interesting note would be the lines of code (or some other size measurement) read in a meeting compared to the time of the meeting. Research has found that the speed of a typical code review is 150 lines of code per hour.

With any of these metrics, it's then important to understand their impact on the process. Root cause analysis, using techniques such as why-because, Five Whys, or Ishikawa diagrams can be used to identify the reasons why code reviews (or any other quality improvement technique) are (in)effective.

You might also be interested in this article about inspections from The Ganssle Group and an article by Capers Jones in Crosstalk about Defect Potentials and DRE.

2

While largely it is true that code review would pick up problems which are rather latent that testing may or may not catch. However, in my opinion you may have a really stable (practically bug free) code but still written in such a way that it is extremely non-readable or not quite maintainable. So it may be that code review may NOT find bugs if there are no real issues actually in the code.

Having said that, i would really ask, why would one want to do code review? The simple reason why it is important is that the code should be improved to be made more readable, maintainable and evolvable. Many people should be able to read cleaner code and make sense out of it. In that sense, simplest objective of the code review process is to produce clean code. So the measure of effectiveness is how much cleaner the code is now.

As you wanted to have a measurable effectiveness - here is what i would suggest:

  1. Metric related to amount of rework - Number of time the rework is applied in a same given module/object/work item is a measure of how poor that code is in terms of maintainability. When effective code-review is applied, how often are we able to reduce the re-work request on the same module?

  2. Metric related to amount of change that every change request incurs. When every time a change request occurs - a poorly factored code will always have larger number of modules get affected. A measure would probably indicate that after a code review - a was that any reduction of such spread of change for a similar change request in the past?

  3. Metric related to average speed with which a change request can be responded. When the code is cleaner - faster and better it is to respond to required changes. After the code was cleaned in the review process, die we find any speed up in responding the similar size request.

I am not putting exact units of measures - you can probably craft more accurate measure about this from this approach. There can be more extension formalism in the above approaches on this.

Basically, my point is that instead of looking at the number of bugs the code-review process identifies; we should measure the effectiveness in terms of whether code-review has been able to bring code to be more cleaner, leaner and easy-to-maintain state; hence, we can gauge that effectiveness if we see that similar change requests in future becomes more efficient to be responded.

7
  • 1
    Although not a "bug", a lack of readability, maintaibility, or evolvability is a defect in the code and should be treated as such. There's no reason why these can't be tracked in a defect tracker, right along with actual bugs in functionality. By doing this, you also open up the ability to track a number of other defect-related metrics in the coding phase.
    – Thomas Owens
    Jan 12, 2012 at 18:41
  • 1
    As a developer I sure like to see clean code. However, code reviews are very costly. So as a manager funding a project, clean code is really not a compelling reason to add 5-10% in costs and time to my development budget. Especially when (as a manager) my bonus/review is tied to completing the current project on-time/in-budget. So your opinion that the main reason for code reviews is to get clean code would make any good manager say the ROI is not worth it. You can argue about long-term returns, but by then the manager that delivers on-time/on-budget will have been promoted away from that prob
    – Dunk
    Jan 12, 2012 at 20:22
  • ...problem. While the manager who promoted the code reviews will have successful maintenance projects but will have been reamed for not completing the original project on-time/in-budget like the manager who didn't. OTOH, if the code reviews helped find bugs that the lack of review didn't and that let the code review manager complete his project more on-time/in-budget then it is a different story. That is the story that needs to be sold. Which also means that clean code cannot be the reason for code reviews.
    – Dunk
    Jan 12, 2012 at 20:25
  • @Dunk The cost of a code review depends on the type of code review. A formal inspection with 3-5 readers, a moderator, and the presence of the author (5-7 people in a room) is expensive. A desk check that consists of another developer glancing over the code for 10-15 minutes is also a code review, but much less formal and much cheaper. Even pair programming can be considered a "code review" technique of sorts. The appropriate technique is determined by factors including (but not limited to) the criticality of the code, the desired defect rate, and the amount of time/money to be invested.
    – Thomas Owens
    Jan 12, 2012 at 20:43
  • @Dunk - I think you've made an argument for taking the "should we code review" decision out of the hands of the project manager, and placing it in the hands of the manager who has responsibility for the software platform long term. IMO, generally speaking, spending an extra 5-10% on development for decent code reviews is a worthwhile investment in terms of the longevity of the system being developed. But probably not in terms of the budget and timeline of the current project. Jan 12, 2012 at 21:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.