Talk About Quality

Tom Harris

What’s the difference?

with 2 comments

What kind of tool is required for code review? What are we trying to accomplish? How much of the code must be reviewed at a time, and why? These are some of the questions that we get caught up in, that prevent us from deploying effective code review in a development organization. One way to cut through the arguments and move forward is to realize that there are different types of code review, with different goals. Here are some parameters of two major, but different types:

Code Review type 1

Key question: Does this change accomplish what it was supposed to, while doing no damage?
Desired code author response to comments: Revise changes
Under review: changed files with diff
Tool: diff comparer
Required feature as a code review tool: right-click submit comment
Nice-to-have features (but could be in a separate app): right-click definitions

Code Review type 2

Key Question: Is this code designed well, so that it is easy to fix and extend?
Desired code author response to comments: Refactoring / renaming
Under review: any subset of entire codebase, starting from a given area of the code
Tool: source code browser
Required feature as a code review tool: right-click submit comment
Nice-to-have features (but could be in a separate app): code editing

Written by Tom Harris

November 17, 2009 at 12:01 pm

Posted in Code Review

2 Responses

Subscribe to comments with RSS.

  1. Good article! I think another factor to consider is the frequency of each type of review. The ‘diff-based’ reviews are the type of review that might be done multiple times per day, so your tool needs to make these fast (e.g. keyboard shortcuts, one-click comments, iterative reviews to address fixes as they are made).

    The ‘design reviews’ are probably done a few times throughout the project before release, and so the ability to ‘deep dive’ quickly into the code, from either the IDE or a Web browser is important.

    Jesse Gibbs

    November 17, 2009 at 9:10 pm

  2. Another way to think about these two types of review, and this relates to their frequency and thus the time allotted to them, is to see them as two complementary aspects of code review. Reviewing the diff is reviewing the “what” — what does this code or change do? Reviewing the design is reviewing the “how” — how does this code or change do it, and how does it leave the code afterwards for future changes?

    When we look at it that way, we can see, as some others have commented to me offline, that both types of review should happen every time, though in different proportions depending on the circumstances.

    As far as methods and tools, we can conclude that the method is one, with different emphasis depending on the situation, while the tools can differ slightly. If the focus is on the diff, then being able to comment from the diff viewer is important. If the focus is on the design, then being able to comment from the full code browser is important.

    Of course, as someone pointed out to me elsewhere, regarding IDEs, it must not be a blocker to have everything in the same tool. Any code review comment takes enough thought that the time to “alt-tab” to another window should be small in proportion to the time it took to generate the comment.

    Tom Harris

    November 20, 2009 at 11:11 am


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s