Understanding Why Peer Review Isn't Just About The Code

  Mai 21, 2015

You've likely heard about the mess at healthcare.gov, and probably know that the website had both functional bugs (mostly from systems that couldn't integrate) as well as performance bugs at go-live.

There were other, deeper bugs you might not have read about.

For example: The requirements were apparently vague enough that they did not say what error pop-ups and other text would say, so the programmers wrote "To Do" and "Lorem Ipsum" right into the text on the website.

This wasn't a programming problem. If it came up in code review, the other programmer, or manager, would likely have said the project “needed to get done” and this was a “reasonable compromise” while we “figure this thing out.”

Then, on October 1st, 2013, the site went live anyway.

If code review couldn't solve this problem, how about consider reviewing the following:

  • Requirements
  • Design Document
  • Workflow
  • Copy-writing and other textual pieces

Any of these might have stood a chance at making the mess, less of a mess.

Companies Require Code Review or Pair Programming

There’s a problem with the way we think about the work, one that we’ve see in the healthcare.gov example, but this problems also affects many American businesses. People who have less than 100% of the information needed to do their jobs are assigned the work without follow up.

Many companies require code review or pair programming and this is where we tend to see this problem. But what about everything else – what else should be reviewed during the development process?

Dr. Barry Boehm is a legend in computer science circles and has conducted a study of thousands of software initiatives to find what commonalities among successful and failed software projects. In his book Balancing Agility and Discipline: A Guide for the Perplexed, he notes that 60% of defects found in production could have been prevented by simple peer review. That starts to put the economics of pairing into a new light considering the cost of repair to a live system can be hundreds of times the cost of changing code before checking it in for the first time.

If you've ever seen a graphic that just didn't quite feel right on a application, it was likely created by one person as an afterthought. Everyone on the team knew the image was not quite right, but each member of the team thought the fix was someone else’s responsibility.

In my experience, I've seen issues where certain issues were not caught because certain information was not in the requirements documents.

Years ago I had to create a report for medical benefits. The report did not make sense; it had a lot of plain text, combined with statements like “Your copay is ($2/$4/$6).” I emailed the project manager and said I was confused, and she told me the language was approved by a government agency and it was too late to change. When they actually saw the reports, they were floored. I was supposed to substitute a single dollar amount in for a certain field in the database.

Dr. Barry Boehm, Balancing Agility and Discipline: A Guide for the Perplexed

The solution to the problem was a peer code review tool that includes peer review for artifacts. Comments are associated with a particular line of code (example: this specific # line of code – “does not make sense to me”) and every piece of code must be reviewed and approved to go forward. In Matt’s case he could have commented by highlighting the specific piece of text and commenting “I do not see where this information is in this document.” The situation could have been handled by leaving the requirements in an unfinished and unapproved state before being allowed to move on in the development process.

Requirement Documents and Manuals Shouldn't Be Reviewed Via Email

So why are we not reviewing everything?

Because it’s impossible to review every little detail.  For example, no one is going to review items like project emails.  But critical information such as requirement documents and manuals should not be going out through email to get reviewed. As we all know, things can get lost in your inbox. The challenge will be to review just enough of the documents that matter; to keep the information that matters in those files, and to limit them to just the information that matters, to prevent the needle in the haystack problem.

First Class Artifacts

Kent Beck, a co-creator of Extreme Programming, explained the idea this way in the year 2000 his book:

Travel light - You can’t expect to carry a lot of baggage and move fast. The artifacts we maintain should be: few, simple, and valuable.

Kent Beck, Extreme Programming Explained

The Extreme Programming (XP) team become known as intellectual nomads; always prepared to quickly pack up the trends and follow the herd. The herd in this case might be a design that wants to go in a different direction than anticipated, or a customer that wants to go a different direction than anticipated, or a team member who leaves, or a technology that suddenly gets hot, or a business climate that shifts.

Like the nomads, the XP team gets used to traveling light. They don’t carry much in the way of baggage except what they must have to keep producing value for the customer - test and code.

It’s been fifteen years since Beck suggested just tests and code, a concept he later expanded in the 2007 second edition. Over time, his definitions of first class artifacts changed over time, but he had a great understanding from the very beginning.

These artifacts should probably live in a version control system, and you might want to make sure they are created through some collaborative process, pairing or mobbing or review required?

Start with the question: what files matter? You may want to also consider the following question: What other information am I missing that also needs to be reviewed?

Once you have the artifacts, continuous review is the next logical step.

Related Articles

Improving Embedded Software Quality with Peer Code Review

How to Grow Agility through Code Review

Defining Code Quality