What Code Review Should Not Find

I write so much about code review that I may forget to emphasize the obvious.

First, “code review” means different things to different people. I mean only that

Someone, who knows good code when s/he sees it, reads the code and gives the code author helpful comments.

Second, code review is not a goal — only a means to an end. The goal is better code now, and better developers as time goes on (which means better code in the future).

That said, I can get to today’s point, which is:

Code review is expensive, so use it wisely.

Do Not Use Code Review

Do not use code review to ensure that fixed text, such as copyright notices and authorship headers, are correct. For those, create code file templates and use them.

Do not use code review to find text layout problems. Instead, adjust your code editor so that it helps you lay the text out according to your style guide, and only lets you lay it out that way. In fact, skip the style guide — write a code editor configuration file, and perhaps some macros, instead.

Do not use code review to find where the language has been used incorrectly or unclearly. The person who wrote your compiler did that work already. Just turn the compiler warnings all the way up, and maintain zero compiler warnings. (And as Gerard Holzmann says in his rule #10, “The rule of zero warnings applies even when the compiler or the static analyzer gives an erroneous warning: If the compiler or analyzer gets confused, the code causing the confusion should be rewritten.”

Do not use code review to look for functional errors. Write unit tests instead. Not only is the CPU faster than a reviewer, and never gets bored, but the activity of writing unit tests often helps the code author see functional errors him/herself.

Do not use code review to find misuse of the language where the construct is error-prone. Consider the costs and benefits of moving to a language more suited to your environment, product, and developers, and if benefits outweigh costs, migrate to the better language.

Finally, do not use code review to find more instances of a problem if root cause analysis of past failures has already identified the problem as serious and systemic. Correct the problem first.

What’s Left for Code Review?

After all of the above is being addressed, then the valuable time of your code reviewers can be invested identifying readability, maintainability, and extensibility issues. For the most part, it takes a person to identify these issues. The reviewer is standing in for another developer—the one who will have to work on the code next.

These issues are the ones which affect how long it takes to fix, change, or extend the code, and how often fixes are required. The time saved by code that addresses these issues is what your time investment in code review is supposed to return, hopefully several times over.

Advertisement

Continuous Code Review

Continuous code review is reviewing all the code, all the time.

That’s the theory. In real life, some questions come up:

How do we track which code has been reviewed?

All the code?

And when is all the time?

Question 1: How do we track which code has been reviewed?

Answer 1: One cannot, and need not, mark code as reviewed.

One cannot, because, for example, if one reviews a file when it has 100 lines of code, and then later it has 120 lines of code, one may have to re-read all 120 (probably more quickly since much is not new) in order to comment.

And code is not reviewed line by line, but idea by idea.

Use a code editor that maintains a symbol tree so that the reviewer can jump from use to definition and back.

One need not mark code as reviewed because all code should be reviewed.

Question 2: All the code?

Answer 2: Yes, all the code that has been included in continuous code review.

During start-up, one might mark components or modules as having been included in the continuous code review process.

In steady state, by definition, all code is reviewed.

Question 3: When is all the time? Pair programming? Or surely, before every check-in?

Answer 3: None of the above. Try once a week.

The idea is to direct a constant flow of intelligent, objective comments to the developer, as soon after coding as possible.

Code review is not testing, and it is not certification. It is a tool to help the developer fix (improve) the code s/he has just written, to improve the code s/he is about to write, and in general, for the developer to become better at detailed design and coding.

Pair programming is not code review because the “non-driver” is not objective—s/he is part of the pair.

And it does not matter whether code is ready for check-in or not.

All you need is enough newly-written code for a reviewer to understand what the developer is trying to accomplish.

Get started!

The Carbon Rush

Rarely does a topic stir up more controversy than code review. And global warming doesn’t either, though perhaps it should.

I was grateful for the opportunity to see Al Gore’s very slick hour-and-a-half documentary, An Inconvenient Truth, about global warming and carbon emissions. Whatever you may think of Gore, blatant promos for Apple laptops, or computer-generated tear-jerker sequences of drowning polar bears, it is worth seeing as a thought-provoking presentation.

Remember to think.

Some of the questions I was left with after recovering from the “mild thematic elements” (yes, that’s what gave it a PG rating) are:

• What does “carbon neutral” mean? And “carbon offset”?
• Are carbon subtraction programs working as planned?
• Is carbon emission reduction going to save us from a global warming disaster?

Nothing is as simple as it seems.

Carbon neutral refers to “calculating your total climate-damaging carbon emissions, reducing them where possible, and then balancing your remaining emissions, often by purchasing a carbon offset. (Related term: carbon negative.) “

(If anyone has a more formal source than a word popularity article, let me know.)

Hmm. What about “carbon negative”? It’s easy to get distracted on the web, but the only real answer is planting more trees. (All that stuff about buying carbon dioxide emission reductions from other organizations is nonsense — not that it’s not helpful, but it’s not taking carbon out of the atmosphere – just paying for someone else to put less in rather than reducing one’s own emissions.) A bit disturbing to find, then, that even Friends of the Earth has raised concerns about how carbon-reduction tree planting is being carried out. Humbling too—one more example of how we never quite know the effects of one man-made intervention carried out to mitigate another.

Finally, if one can entertain the thought that world climate change might be a bit more complicated than a 100-minute movie, there’s short, medium, and long reading to be found at JunkScience.com.

What seems to me, though, is that the reductions—that are currently fashionable to call carbon emission reductions—are probably good for “old-fashioned” (1960’s) reasons: reduce landfill, keep air clean for breathing, improve personal health through exercise, and increase spiritual health by slowing down.

Will all this get lost in the Carbon Rush?