I was recently at a networking event chatting with a non-technical acquaintance.  I mentioned that I was mentoring a new member of my team who was going to start reviewing some of my code soon.  I analogized that no one would consider publishing a book without an editor, and code needed a second set of eyes as much or more than prose did.  It got me thinking about how publishers have style guides that can be used as references when editing and I needed some way to explain to this young developer what to look for as he began to review my work.  Some teams have rigorous style guides and certain languages have favored idioms, but I was looking for something more generic and ended with these three broad guidelines.

Is it Correct?

If an article about Abraham Lincoln mentioned “the president’s wife, Martha” an editor might recognize the wrong president’s wife was named and easily make the correction.  Unless the article was about Presidents’ Day or some other topic that discussed the first and sixteenth presidents, then the editor might have to go back to the author and ask for a clarification.  Similarly, some code is clearly wrong on its face and some less obviously so, but in both cases careful reading by a fresh set of eyes can reveal many — but likely not all — the places that the code is incorrect.

Is it Consistent?

Sports writers have a special talent for finding hundreds of different ways to say “won” or “beat.”  Writing software isn’t — shouldn’t be — nearly as creative an endeavor.  If you create one object you don’t add another, unless that difference in term indicates a difference in use.  If “close” is the opposite of “open,” it is always the opposite of open, never “shut” or “seal” or “drop.”

Is it Complete?

When the anchor of the nightly news tells you that a stock market index is up 200 points today, what do you make of it?  Nothing, really; you have no information to go on.  Numbers with units — points in this case — are meaningless.  A 200-point swing in the Dow is very different than the same change in the NASDAQ.  Without a basis for comparison — like the value of the index — the story is incomplete.  If you were told it was up or down 5% or 15% then you have reason to cheer or worry.

What if your favorite sports team’s rival played yesterday and you wonder how the game turned out.  The sports section may be full of scores and commentary, but if the game you are interested it isn’t covered, the section is incomplete.

Putting it All Together

Software can be complex and subtle and reviewing it is not trivial and must be done on several levels.  As an analogy, consider the recent sales listed in the real estate section of my Sunday paper.  There is one item after another like:

Alice and Bob Smith bought property at 704 Main Street from Ted and Carol Jones for $195,800.

There are hundreds of these items county by county (in alphabetical order) and town by town within county (also alphabetical).  The items themselves seem to be unordered, perhaps they are by transaction date.

Is it correct?

The names of the parties, the address, and the price in each item are verifiable.  There is a record of sale somewhere that can be consulted to be sure that the facts are correct.  If a few letters in the middle of the listing were in italic or bold or a different typeface, you could reasonably argue that is incorrect, too, though at a different level.

For software, the source of truth about the software’s intent is the design, sometimes reflected in a test it is expected to pass.  When revising software to address a bug, you must reference the ticket where the bug is described to know what the wrong and expected behaviors are in order to assess the correctness of the code.

Is it Consistent?

Occasionally the real estate sales include an item like

Betty and Barney Rubble bought property at 1 Gravel Lane from Acme Relocation Services for $1,234,567.

This is noteworthy because the seller is a company.  More subtly, you might also have noticed in the first example that one couple is listed woman first and the other man first.  Is that intentional?  Perhaps the listing just echoes the names as they appear on the deed.  Or perhaps they are intended to always be in alphabetical order (in which case the Ted and Carol are in the wrong order).  Once you are accustomed to the standard format, exceptions stick out.

This happens with software, too.  If two pieces of code do very similar things, they should differ only in the ways that are necessary.  Unexplained or unnecessary differences should cause the reviewer to pause and wonder if they are also unjustified.

Is it Complete?

On a rare weekend, the listing of sales omits my town.  It is hard for me as a reader to tell if single transactions are missed with more regularity.  If I were tasked with editing or reviewing that section I would need a checklist of all the communities covered and their counties.  That list could be used week after week.  I’d also need at least a count of transactions per town and overall as a first check of completeness.  And if accuracy was paramount, I’d want some form of the sales records to reconcile the listings with.

Even a simple software system may be built around several different classes.  For classes that may be stored as records in a database, each needs code to create, read, update, and delete instances.  Your classes have to cover all those operations for all the classes (or your comments need to explain why, say, one object is immutable after creation).

That system should have a test suite that covers creating an instance of each class in each way that is meaningful (using default values, using overridden values, using invalid values, etc.).  It should test retrieving or reading each class, including trying to get one that doesn’t exist.  It should test updating an instance of the class with no changes, with valid changes, with incompatible changes to values, etc.  It should read and attempt to update one that is deleted between the operations.  It should test deleting objects, including trying to remove one that doesn’t exist.

The combinatorics add up quickly — like the sales in towns and the towns in counties — and reviewing the code of the implementation and the tests that verify can be painstakingly detailed work.  But if the system is important enough, it is necessary.

That’s my 1,000-word take on three watch words for code review.  Are they enough?  Probably not.  But I hope they provide a starting place for novice reviewers as they develop their reviewing skills.