I had the pleasure of spending some time talking about project management and software development with James Grenning, who was in town to do training for Object Mentor. (You can read his articles here.) We spoke about a wide range of topics, and I definitely learned a few things about code reviews, user stories and documentation.

James had some very interesting ideas. We spent some time talking about code reviews. He pointed out that when selecting code to review, it makes sense to concentrate on code that links objects together, rather than internal behavior of an object, since that’s what unit tests are for. That made a lot of sense to me — the unit tests Lose Weight Exercise the objects themselves, which means the risk of defects shifts more to the integration of objects. Targeting the reviews at the integration code is much more likely to catch exactly the sort of defects that unit tests would miss. (Note to James: I hope we get to see an article or blog entry giving us some details on this!)

The most interesting thing we talked about revolved around requirements gathering and XP user stories. I’ve long been of the opinion that while user stories can make good requirements gathering tools, they seem too temporary. They’re written on 4×6 index cards, and they never seem to be used again once the system is built. James pointed out that this is the point of user stories — they’re made permanent once they’re turned into acceptance tests. And then he said something very interesting: he pointed out that this eliminates the need to store redundant information about the project.

I think this is where James and I may have reached a disagreement. I think there’s a lot of value in having what might seem to be “redundant” information in different project documents. This is definitely counterintuitive for many programmers, since we’re trained to only store information once, and reference it rather than duplicate it. I think it’s very valuable to have, say, a rationale in a use case replicate some of the information that might be in the vision and scope document, or have a functional requirement mirror certain steps in a use case. Often, it’s useful to repeat a piece of information in order to make a requirement or use case more readable. And most of the time, that information isn’t necessarily redundant. A requirement may go beyond what’s written in the use case, adding information about how the software should function in order to implement it. When this requirement is reviewed, that extra information can provide a valuable quality check — if one of the team members has a misunderstanding in that area, it will make it likely that the two douments won’t reconcile with each other.

I think the basic principle at work here is that documentation isn’t created for its own sake. In some cases, like with user stories, the documentation is only an intermediate work product that leads to code and acceptance tests. Once those things are created, there’s no need to keep them around. On the other hand, project documentation like a vision and scope document or a use case has a different purpose: to make sure that everyone on the project team has the same understanding of what the project is supposed to do or how the software is supposed to behave. And, as we all know, while it’s more work to keep this sort of documentation up to date (through reviews, inspections and change control), this gives the team a lot of opprtunities to find and repair defects before they make it into code.

Have you ever noticed how hard it is to get people to do code reviews? It seems like developers and project managers alike are allergic to them. I’ve noticed that when I talk to people about code reviews, the same few objections come up over and over again. I think it’s worth looking at those objections, and why they don’t really make sense when you think about them.

Myth: Code reviews are a waste of programmers’ time.

For some reason, many programmers and project managers will shoot down code reviews, claiming that they’re a waste of time. In my mind, if a practice is a waste of time, it means that practice doesn’t save the project more time than it it costs to perform. And in my experience, this just isn’t true when it comes to code reviews. When I hold code reviews, I often find that the team ends up with a sense of relief because we caught a bug which would have been far more work to track down later on.

Typically, a code review with four people will requie about ten person-hours: each person spends about half an hour preparing, and the meeting lasts about two hours. In that time, we almost always identify a couple of defects which we probably wouldn’t have found otherwise. Everyone is keenly aware that had those defects been caught by the testers (or, even worse, the users), it would have taken a lot more than ten hours to track them down. It’s not uncommon for a code review to start out with a bunch of groans, only to end up with all smiles!

Of course, “data” isn’t the plural of “anecdote”. However, my individual experiences are borne out by many studies, which have found time and time again that code reviews (and, in fact, most accepted inspection practices) pay for themselves.

Myth: Code reviews aren’t effective because you can’t review all the code.

It’s true that code reviews cannot cover all — or even most — of the codebase for all but the smallest applications. Since code reviews can only catch defects in the code samples being reviewed, doesn’t this mean that they’ll only catch a tiny fraction of the defects?

But just because you can’t review all of the code in the project, that doesn’t mean code reviews aren’t worth doing! It turns out that if the right code samples are selected for review, then code reviews can be very effective. In most projects, a large number of defects tend to be concentrated in relatively small pockets of code. If the code is selected well, then the review will catch the most costly defects.

Note: You can use this code review checklist to help select the right code sample.

Myth: Given a chance, programmers will endlessly “tinker” with the code.

This seems to be one of those ideas that only managers come up with. I’ve never met any experienced programmer who would think of using a code review to “tinker” with code. It turns out that a code review is a pretty poor forum for some sort of mindless code “beautification”. Reviewers are not engaging in a top-down redesign of the code — they only bring up problems which they believe are defects. What’s more, the collaborative aspect of the code review meeting tends to discourage any changes that are only made for the sake of making changes. (It turns out that software developers are a pretty pragmatic bunch, and they have a low tolerance for useless behavior.)

Still, it’s good for the meeting moderator to be aware of this myth. That way, he can cut short any discussion which he thinks is leading down that path.
Myth: Programmers aren’t paid to sit around and talk, they’re paid to code.

There’s a great story in Peopleware by Tom DeMarco and Timothy Lister (which is a fantastic book that everyone involved in project mangaement should read). A programmer is sitting at his desk with his feet propped up, staring into space. His manager comes by and demands to know what he’s doing. The programmer says that he’s thinking. The boss asks, “Can’t you do that at home?”

Software development — in fact, any engineering activity — requires a lot of thought, and a lot of discussion. It’s rare for programmers to be given the time that they need to discuss the difficult problems that they routinely encounter. A code review is a great opportunity for a project manager to encourage that sort of discussion.