The Purpose of Code Reviews

Question

As a Manager ultimately responsible for hundreds of developers, I believe that code reviews are required to ensure:-

1. That code produced is maintainable.
2. That published programming conventions/standards are met.

In my circumstance, they are not to verify operation or results, as formal testing will quickly determine that.

The question that I ask reviewers to primarily keep in mind, is whether or not a new average programmer will be able to understand the code in a few months time, and specifically whether there is sufficient code clarity and comments to support that.

Is this what most programmers understand is the purpose of a Code Review?

Answer 1

Yes, but they can also:

• Help to find subtle bugs (such as not correctly implementing IDisposable, etc),
• Provide a form of mentoring, so that knowledge/expertise/techniques are dispersed amongst team members
• Examine and improve design decisions

Answer 2

One important aspect of code review is a means for developers learning from each other, sharing knowledge and expertise. This helps each developer grow their experience.

Answer 3

It's worth training your reports in code-reviewing! See this video interview of mine for a brief summary, and the slides of my talk's presentation for more details.

Answer 4

We use code reviews for a lot of reasons:

1. To bring someone new across the code
2. Ensure maintainabilty (layout, comments, adherence to standards)
3. Catch edge-cases (you'd be surprised how often this works)
4. Find areas of improvement
5. Share Knowledge

The main 2 reasons are the first 2. I find collaborative development to be better than reviews, but in the absence of that, reviews are good.

Answer 5

I don't understand your reasoning about reviews not verifying operation or results. If a reviewer sees a bug (i.e. the code will give the wrong results) why would they not make that part of the review?

Reviews should absolutely be about correctness as well as maintainability and standards. The earlier you catch problems, the less time is wasted testing the incorrect code.

For me, reviewing is about (in no order):

• Ensuring code standards are adhered to and that the code is "clean"
• Suggesting alternative approaches (whether at an implementation level or design)
• Ensuring there are sufficient unit tests, and perhaps suggesting others
• Ensuring that the code appears to be correct (if there are enough unit tests this is likely to be the case, but there are occasions where a set of tests which looks sufficient misses something)
• Learning about the code base to make it easier to maintain later on through familiarity
• Encouraging discussion between developers so both parties can learn

Answer 6

This seems a little too limiting.

I've participated in code reviews where an experienced programmer has pointed out incorrect library use (doing things the hard way) and problems with basic design. This has proved very useful.

However, it seems like the value of a code review depends on the group that is doing it. In some cases, with some programmers, your stated purpose could be perfect.

Answer 7

More important than those, code reviews can:

• Find bugs!
• Find bugs early. The sooner a bug is found, the less it costs to fix. A code review can be done as soon as the code is written.

We use code reviews and they are very helpful, but they often get done too late in a release cycle (for instance after most other QA work is already done) when fixing bugs is expensive, so bugs found sometimes don't get fixed (or get fixed less optimally).

Answer 8

If you are already performing code reviews for the two reasons you listed, then it's an easy win to allow the reviewers to pay attention to the other possible goals of code reviews listed by various commenters. I've found that reviewers of my code often detect bugs and have suggestions for refactorings that result in better, easier to understand, code. Experienced reviewers have taught me a lot. I've found that being the reviewer also taught me a lot: not just about 'ways to do things', but also about 'how to communicate criticism'. Code reviews are extremely worthwhile.

Answer 9

Yes. I'd say that is a primary thing to bear in mind.

• It is difficult to verify that code is well written (algorithmically) in a code review, unless you can devote a lot of time to the review. However, you can quite quickly and easily check that code is well formatted, well documented and easy to read and understand. You can often pick up on a number of poor programming practices too (copy & paste of code blocks rather than using a shared subreoutine, etc)

• Programmers can learn a lot from doing code reviews (both giving and receiving). By looking at each others' code, experienced and inexperienced programmers alike can learn from each other (coding style, cunning tricks, as well as improving general knowledge of how systems work in your application)

• One option is to allow coders to prepare code prior to reviews. In one sense this means a sloppy coder can "hide" his sloppiness from the review, but of course it also means that his reviewed code is brought up to standard and he learns how to write better code. The question to ask here is: are you seeking to "punish" bad behaviour, or simply to encourage good behaviour.

• Try not to get too "nit-picky". The important thing is to encourage and teach programmers how to write better more maintainable code and work better as part of a team. If programmers feel they are being picked upon it can have a negative effect. Ask a programmer to identify what they can improve rather than pointing out all their faults. After a review, give the programmer time to revisit the reviewed code and correct any problems you identified.

Answer 10

My code review values (sorted in descending order of importance):

1. Code does what it is supposed to do: algorithm is appropriate and correctly implemented.
2. Design is good enough: the code is easy to use, easy to test, easy to expand.
3. The code follows the company style guide.

Sharing of knowledge is a very nice side-effect, but not an "essential" purpose of the review. Typically, the review should be done by someone who is in expert in the area (as much as possible). All the people that need to read the code should be cc:ed.

Answer 11

I don't think that standard methods of reviewing code is 'that' useful. I wrote a small post a while back regarding the usefulness of code reviews -

"Rather than do a code review at every checkin of the code, it is often better to do a architecture review at certain points of time. For example, when you are planning a new version of the product, get your best architects to review and discuss the existing architecture with the development team. An architecture revamp before you start work on a new version ensure that the product as a whole stays in shape. People are also okay with doing the review at this higher level and developers are eager to take on the challenge. "

Answer 12

A Code review should not consist only in reading other people's code but also taking time for a meeting with both developer explaining what he did, and reviewers asking questions.

This way, the developer has the opportunity to find flaws in his own code, when his explanations don't convince himself.

Answer 13

As a programmer, I think the purpose of code reviews is so other developers can check for weaknesses in my code and thus to provide feedback so I become a better programmer. I have reviewed plenty of code of other developers too in my organisation. Checking if code will work is done by the testing department. I make sure, during reviews, that they chose to write good optimized code. (Plus, readable code.) Code reviews help to share experience amongst the whole team. I don't think that you should review code to make sure an average programmer will be able to understand it all, since some code can be quite complex. I tend to work on the more complex algorithms in our projects, and have to make sure those algorithms have a good performance. As a result, I have to write complex code that even the more experienced programmers won't understand at first, no matter how well-documented I make it. I had one programmer review my code once, thinking it could be rewritten with a lot less code, so he did. As a result, a process that took 2 minutes at first suddenly took 45 minutes to finish, leaving that programmer very confused since he didn't realize at first that my code was heavily optimized for speed.

Code reviews are very valuable, but don't do them to keep the code readable. Use them to check for less optimal code.

(Of course, code should be readable, variables and method names should be chosen well, parameters need to be clear, comments are a requirement, with at least one comment per method but preferably with at least one line of comment per line of code. The source needs to be well-formatted, divided over proper namespaces in a clear folder structure and additional documentation need to be readily available. But this I don't even consider part of a review, but part of common sense.)

Answer 14

Many thanks to all that answered this. Because this question is subjective, there likely isn't a "right" answer.

Perhaps most interesting to me personally, is seeing that few [if any?] Programmers consider Code Reviews to be about reducing the cost of maintenance - despite it representing the highest proportion of cost over a non-trivial projects lifetime. But this is probably consistent with the subtle perspective in the comments that programmers somehow own the code.

Given a very large commercial software system, the reality is that the code base is a separate entity serviced by programmers of varying skills, who will likely come and go long before the system is discontinued. Software Reviews are ultimately there (IMHO) to serve the code base, not the Programmers.