Talk About Quality

“Britain’s Ugly Duckling Breaks Out in Song” I slowly translated off the showbiz page of a foreign-language newspaper. I had to work at it to figure out that it said “Susan Boyle” and then look up her appearance on YouTube from a week ago. Anyone who wants to be moved by song, and doesn’t mind (or enjoys) the contrast with “beautiful people” celebrity judges putting feet in their mouths, should stop and listen.

I thought I would have nothing to add to the commentary on a musical appearance that saw over two million views (and that’s just on one upload, let alone the original TV broadcast). But after reading the 5-day-old Wikipedia entry, and some of the newspaper articles in the references, I wondered why nobody offered the obvious. Good singing comes from interpretive ability, soul, and poise. Anyone who has enjoyed opera would have no reason to be surprised–and every reason to be moved–by Boyle’s voice. Similarly by that of her “predecessor” Paul Potts. It is the furthest thing from coincidence that Potts sang opera, and Boyle sang from a musical, both genres that are formally performed live without a microphone.

So, while Tanya Gold and others may not be wrong in their social analyses, they are missing the point. Song, in contrast to the child of yesteryear, is to be heard and not seen.

Why Code Review?

We write code for two audiences. One audience is the hardware: the compiler, and the platform where the executable runs so that the user can operate it. The other audience, no less important, is the “next developer”: someone else (or even yourself a few months later) who has to modify the code to fix it or add new features.

The hardware’s reaction to the code is clear from testing. But the only way to find out if the code is easily maintainable is to have someone else—a knowledgeable developer—read the code. And the earlier, and more often, the better, since errors can be caught sooner and cheaper as well.

Additional benefits of code review: improving the code author’s coding (footnote 1) through the experience of guided correction, and finding and preventing systematic errors.

Code Reviewer and Code Author are Partners

The code reviewer must know the programming language well, be familiar with the technology (footnote 2) that the code implements, and be patient and effective in giving constructive criticism. The code author is the current maintainer of the code. Both must see themselves as working together to improve the code.

Review Comments, Reactions, Tracking to Closure

The code reviewer reads the code and makes comments. The code author responds by changing the code to improve it according to the comments s/he accepts. All comments must be recorded, electronically, both for tracking to closure, and for later review for systematic errors. If the author makes some changes during the review meeting, a “diff” of those changes may substitute for a list of those accepted comments.

First, Leave the Reviewer Alone to Read the Code

The reviewer first reads the code, alone, in a code browser that maps, and supports easy navigation of, the code’s structure. This method best simulates the “next developer’s” experience: the code author does not come packaged with the code—it must stand on its own.

Afterwards the reviewer meets with the author. If there are multiple reviewers, they may meet together with the author in a group review, but each reviewer must have first reviewed the code alone. A meeting should be in-person, but may be (less desirably) telephone or electronic, as long as it is interactive.

What Code Author and Code Reviewer Provide to each other

The author makes a labeled version of the codebase available to the reviewer. The requirements for the new, improved, or fixed code, and any required design, should also be known to the reviewer.

The reviewer reads the code for clarity = readability + understandability. Code is readable if it has the right balance of abstraction vs. detail at each level of the code. Code is understandable if the reviewer can, without help from the author, see what the code does, and why. The reviewer may use detailed checklists of common errors, and use the coding standard as reference, but review for clarity will lead to all the others.

When to Review Code: At Minimum, Before Check-in to a Shared Work Area

Code may be reviewed at any time. Here are some different times, and the benefits of review at those times:

• As code is written: Find errors earliest, prevent repetition by improving the author’s performance
• Before static analysis: Avoid analysis time on code that doesn’t belong
• After static analysis: Avoid review time finding errors that a tool could find
• Before check-in: Maintain quality of the codebase
• After check-in: Allows time for thorough review
• Before delivery: Catch errors before testing group
• Before debugging: Narrow down debugging steps; possibly find error without debugging
• After field deployment: Understand problem areas reported from the field; make improvement recommendations

The team, group, or department lead may decide when to review code according to his or her desired benefit. However, if time constraints limit code review to just one of these stages, the required stage is before check-in to a shared work area, to maintain the quality of the codebase that others will learn from and use.

Which Code to Review?

All new or changed code should be reviewed as above, at least before check-in to shared codebase. More complex or historically problematic code—more thoroughly. Take care also to review the new or changed code’s “nearest neighbors” to make sure the code still works with existing code it interfaces with.

Footnotes

1. If there are multiple reviewers, the reviewers can also learn from each other’s review comments. But there are tradeoff costs of multiple-reviewer reviews: variation in quality of review, need for a strong moderator, and more time. Better for reviewers to learn from code review in the code author role, or in code walkthroughs specifically planned for cross-training.
2. There may be exceptions in special-purpose code reviews such as performance or security reviews, where the reviewer must then be knowledgeable in those areas, but not necessarily in the technology.

A co-worker forwarded me this article ”5 Reasons for Software Developers to Do Code Reviews (Even If You Think They’re a Waste of Time)” which certainly sounded promising. Even when I don’t think code reviews are a waste of time. But as I read through it, it became clear that more is less. The article says too much, and detracts from its own message.

1. Developers know their code will be evaluated, so they work harder. “The most useful thing about a code review is the fact that the coder knows that someone is going to review the code,” says Oliver Cole, president of OC Systems and also lead for the open-source Eclipse Test and Performance Tools Platform project.

Work hard because you enjoy it. And of course your code will be evaluated, but not primarily by code review. Rather, the main user of your code is the “next developer”—possibly someone on your team, or even you yourself a few months later. That’s where the evaluation happens.

 2. It improves a developer’s own programming skills.
In your heart, you might not care that much about the success of this particular software project. But most programmers want to improve their personal skills, and that means learning from other people.

If you don’t care about the success of the project, code review won’t help.

3. It’s an opportunity for mentoring, so the programmers you work with get smarter (and thus, more fun to hang around with).” [...] While the intention [to mentor individuals] is generally well meaning, it can often lead to individual discomfort and perceived or actual criticism. In these cases, the greatest opportunity for mentoring usually exists in the context of small collaborative teams working together to realize goals and not in a code review.”

Criticism is not bad, it is essential. It is not personal, but professional. And of course, the smaller the meetings (down to even just 2 people — reviewer and author), the better.

4. It creates consistency and a culture of quality across the project. [...] Developers are quick to complain about being judged on the wrong metrics, but, says Gary Heusner, client partner at custom software developer Geneca, “We have to change the rules to allow for quality and efficient development to be valued over making milestones that are really yardsticks of process more than milestone of value delivered.” Code reviews are a big part of that.

Code reviews are simply part of good software development.  When management, together with the team, track value delivered, that is a big part of creating a culture of quality. Only when the environment is right can code reviews have a chance of being effective.

5. It encourages team bonding. “People think code review is just about finding bugs, but it brings people together, says Smartbear’s Jason Cohen. Often, he says, it can deliver far more than expected.

“Success stories happen very often when performing code reviews,” says Dave Katauskas, senior architect at Geneca. “But the best success story is the pattern that develops once a team has gelled. The longer you’re into a project, the better quality code is created. This is all due to the code review process and governance that occurred up stream in the beginning of the project.”

I had to read this one a few times. Right answer for wrong reasons. I will not credit code review where credit is not due. Even the writer with the “success story” realizes the true origin of the success is the gelled team.

But still, I had to click on the Jason Cohen link to see why code review “brings people together”. Go ahead—click below on “Lightweight Code Review Episode 5: Team Building for the Cold, Dark, and Alone”. But first, get ready to read it right: code review doesn’t create good teams. Rather, good teams benefit from code review. OK, now click.

Lightweight Code Review Episode 5: Team Building for the Cold, Dark, and Alone

In an article Using Static Analysis to Find Bugs, in the September/October 2008 issue of “IEEE Software”, authors Nathaniel Ayewah et al conclude,

We need to develop procedures and best practices that make using static-analysis tools more effective than alternative uses of developer time, such as spending additional time performing manual code review or writing test cases.

In an otherwise fine article, just one key word–alternative–is misleading, promoting a common misconception. Static analysis tools are not an alternative to code review, nor can they replace it. Rather, static analysis tools are a guide to code review, helping the developer better use the time in “manual” code review. Of course, “manual” code review isn’t really “manual” — it’s not about the hands, but about the mind, understanding the code.

When using a static analysis tool, like the FindBugs of the article, or other types of tools, that measure and mark up the code for complexity or code coverage, the benefit is twofold. First, an automatic tool goes through a lot of code, and identifies which parts are most urgent to review. Second, the reports pose challenging questions for the code author or maintainer to answer by re-reading and explaining the code.

Where I work, on some projects at least, there is a formal policy for using static analysis tools. They are run automatically on all builds, and the results are tracked towards targets. But most important is the questions they generate: Why is this piece of code written this way? Is it what we meant? Why couldn’t the compiler, or lint tool, understand it without warning? Why does the code coverage tool think there are so many branches in these few lines of code? All these questions are generated by the tool, and mapped automatically to the code. But it still takes code review to answer the questions, and to adjust the code to make it better.

It’s not hard to find a list of the better-known prerequisites for Continuous Integration. See, for example, Take Advantage of Continuous Integration, in a 2006 issue of Visual Studio magazine. They list:

• source code repository
• fully automated build process
• automated tests

But slap those together and turn it on, and you may still be left with a build dashboard full of “FAIL” messages, and nobody quite sure how to fix them and keep them fixed.

From my recent experience, there are at least two additional requirements. As usual, it’s the implicit requirements–the ones everyone assumes are alread met but really aren’t–that make all the difference. So, continuing the list,

• reliable builds, identical between developer’s workstation and the central build system

Sounds obvious, right? It’s really two requirements that go together, and neither is so obvious in practice.

Reliable: when a build doesn’t always work exactly the same way on a developer’s machine, s/he can re-run it, and may never find the time to troubleshoot why it doesn’t work 100% of the time. But when a central build system runs it, everyone expects a passing build to pass every time. Fail has to mean “something’s wrong with the code”, not, “oops, I guess that build didn’t work this time.”

Identical: the build results obtained by the developer and by the build system must match. Nothing saps faith in a build system like the “WOMB” (“works on my box”) response. (Thanks to Don Hass for that acronym, in his post Who cares if the build is broken?)

And speaking of which, Don was actually commenting on Derrick Bailey’s original notes, The Build Is Broken… Who Cares? Our last prerequisite for today:

• Somebody has to care that the build is broken

Derrick concludes that

“The entire team cares.” If we create an atmosphere where a broken build is a bad thing, then we have created a more productive team.

But to create that atmosphere in the first place, there has to be a first person who cares. Someone who looks at the entire build system display, often with multiple components and platforms, identifies who needs to do what to get each build passing, and follows up daily to get the display all green. It can be a thankless job, but it’s well worth it, to ensure that all the builds are reliable and passing, create that self-sustaining teamwork atmosphere, and make Continuous Integration really work.

This month, CrossTalk, The Journal of Defense Software Engineering, has once again provided a great service, at no charge. (Well, no subscription fee–it’s your tax dollars at work.) It’s their 20th Anniversary Issue, and they’ve brought together some of the real greats to talk about the last 20 years of software engineering, and its future. Even though these thought leaders, among them Watts Humphrey, Alistair Cockburn, and Gerald Weinberg, have all taken different directions over the years, it’s perhaps no surprise that there’s a common thread.

How to Keep Software Engineering Running in Place

• Don’t deploy what works
• Don’t build teams
• Don’t learn on the job
• Ignore the data

If you don’t believe me, take it from the experts: http://www.stsc.hill.af.mil/crosstalk/2008/08/index.html

“Isn’t it nice to know that, when all else fails us, we have an innate decision-making tool to fall back on?”

Robert L. Glass, “Intuition’s Role in Decision Making” (IEEE Software, January/February 2008)

Yes, Glass admits, estimates that come unbidden from a manager’s subconscious seem the very opposite of quantitative or rational. But most decision-making methods have a common theme: using historical data to decide what to do next. Quantitative estimation puts everything out in the open. Rational support for a desired deadline is at least based on the facts. Intuition, at the other extreme, pulls that data, informally known as “experience”, from the subconscious. And since a good manager has experience, intuition works.

Sort of.

What’s hidden by Glass’ essay are the unstated quality standards that justify each method. Only a complete, quantitative estimate, matched by continuous measurement and adjustment, can promise high quality by the target date. Intuition, on the other hand, even coming from an experienced manager, makes quality likely, but hardly guaranteed. His successes are what keep him in his job, and his few failures are forgiven. Rightly so. He is employed to deliver good enough software, quickly, to meet modern society’s ever-growing appetite for computerized life.

So should we favor intuitive, seat-of-the-pants estimating, with its benefit of early delivery, but its cost of uncertain quality?

Digging deeper, we find that there are some very different reasons why we write software, which strongly influence how we plan our work.

1. Profit
2. Functionality
3. Beauty

Anyone who gets paid for writing commercial software must acknowledge that profit pays his or her salary. Profit leads eventually to pretty good quality, via competition. In the short run, though, we experience a lot of pressure, a lot of errors, and many customer-accepted (if sometimes societally unacceptable) failures. In that context, it seems, we should estimate quickly and intuitively, but admit that quality is expendable.

In a well-funded, non-profit organization (e.g. NASA, famed for error-free software — wholly separate from failure-free flights), software can be about complete functionality. Take as much time as needed to implement everything, perfectly. After all, when the spaceship flies past Jupiter, there’s no second chance.

Where, then, is guaranteed perfection, estimated correctly and delivered quickly? The only place we see that in life is artistic performance. A pianist plays, from memory, a piece that she only decided a few months ago to perform. An hour long, no mistakes. The event starts, and finishes, on time. The customer — the audience — rises to its feet in noisy appreciation. The enabler of perfection is seeking beauty, or doing a thing for its own sake.

Many software developers write software because it’s beautiful, fun, or spiritually rewarding. And that reason engenders the highest-quality work. A quality that delivers on time, with no costly rework. Functionality. Profit too.

Somehow, in the commercial context, that reason why we write software must be harnessed and encouraged by software managers. Otherwise, dismissing a developer’s data with an intuitive estimate is not a fall-back, but falling backwards.

Some return for your U.S. taxpayer dollars:

CrossTalk, The Journal of Defense Software Engineering is an approved Department of Defense journal. CrossTalk’s mission is to encourage the engineering development of software in order to improve the reliability, sustainability, and responsiveness of our warfighting capability and to inform and educate readers on up-to-date policy decisions and new software engineering technologies.

Whatever you may think about “warfighting”, CrossTalk sometimes has some really good articles, and even some great ones. Here are two, from the December 2007 issue:

Very good, about software maintenance:

Geriatric Issues of Aging Software

Great, about moving beyond “escaped defects” to defect prevention: 

Advancing Defect Containment to Quantitative Defect Management

Both are thorough and comprehensive—worth reading!

This week, I moved from a general process improvement group to a software development department, to specifically focus on code quality. The most mature tool and process I left behind was a defect tracking system. Of course, in our development department, we use that defect tracking system. In case anyone asks me what it’s for, here’s my answer.

Prevent Errors, Fix Defects, Describe Failures

There are many definitions of those terms, but since when we fix problematic code we close “defects” in the “defect tracking system”, I started with that word in the middle. Leads to these definitions:

Error: The mistake someone made. (The root cause is the earliest error in the chain.)

Defect: The wrong code (missing, extra, or incorrect).

Failure: The unacceptable behavior of the code when running.

Detect each as early as possible

Each can be detected with appropriate tools. Detect earlier to minimize cost and annoyance.

Meanwhile, do it right

How?

With tools, learning, and discussion for writing good code.

When I talk about tools for coding, some people ask,

“How can we be free when the environment is computer-controlled?”

It’s all about the right metaphor. One of mine is woodworking.

“By definition, a jig guides your tool and a fixture guides your work.”

Thus begins Jim Stack in The Best Jigs and Fixtures for Your Workshop. And he claims the 30 best in his book. But here’s a similar example from another source, to give you the idea.

There’s no improving on the authors’ explanation:

“If you’ve ever built a picture frame you’ve experienced the frustration of trying to get all of the parts to fit perfectly. While trying to “dry-fit” and clamp the frame’s pieces together the parts often fit perfectly. But add glue, and a little clamp pressure, and the parts begin to slide apart. It can be a frustrating race against the clock to even things out before the glue begins to set.

(By the way, by Stack’s definition, this example is really a fixture, not a jig.)

If you’ve ever tried framing a picture, you may know what they’re talking about.

Equally well if you’ve ever raced to get your program to compile and run before you go home.

Yes, that picture frame looks pretty tightly constrained. But it’s not limiting my creativity as a carpenter.  It’s just holding the pieces in alignment so they don’t all fall apart. I don’t find myself thinking creatively when I have glue all over my fingers and wood stuck to the carpet.

Your tools include the IDE and the compiler. Your work is, clearly, the code.

What kinds of add-ons (“jigs”) guide your tools? What kind of fixtures can help guide your work?

Here’s the rest of that site: http://www.woodzone.com/tips.htm. Look through it and then make your own list for coding.

You don’t even have to watch your fingers.