Talk About Quality

After spending more time than I should have troubleshooting a coding (actually static code analysis) problem, I shared the story with a co-worker, who reminded me that Kernighan and Pike had been there before:

Another effective technique is to explain your code to someone else.  This will often cause you to explain the bug to yourself.  Sometimes it takes no more than a few sentences, followed by an embarrassed “Never mind, I see what’s wrong.  Sorry to bother you.”  This works remarkably well; you can even use non-programmers as listeners.  One university computer center kept a teddy bear near the help desk.  Students with mysterious bugs were required to explain them to the bear before they could speak to a human counselor.

Brian Kernighan and Rob Pike, in The Practice of Programming

Seems that there are rules that we learn, then love to break, and are always sorry afterwards. Here’s how.

My code doesn’t work

• I’ll just try this change to see if that fixes everything. And then this change, and this other change.
• I’ve been stuck for 10 minutes, but since nobody knows this code like I do, I won’t ask anyone for help.
• I know I don’t understand what every line of code does, but it’s OK — I’ll figure out the problem anyway.
• A complete build takes only a few minutes, so there’s no need to try a 3-line sample instead.
• I’ve read the documentation and I’m sure I’ve understood it, so I won’t ask anyone for a second opinion.
• I already asked someone 3 times for help and got it, so I can’t bother him or her a fourth time.
• It doesn’t compile (cleanly, or at all) just now, but soon it will again if I just keep at it

(The only way I got home today was that somehow I did realize it was, um, OK to call the tool support line.)

What are your favorite ”justifications” for why it’s better to stay stuck than to get help, or at least take a break and share your problem with someone else?

While I’m waiting, I’ll go talk to my teddy bear.

Sometimes I get the feeling that when people consider and talk about code review, they’re talking about different things but they don’t know it. I see code review everywhere during coding.

Try this: put aside the image of code review as a meeting, or even code review as an independent activity. Limit your description of code review to: someone else is reading your code, you’re discussing it together, and there’s effective writing as a result. If you see those three things happening, you’re seeing code review.

Debugging

The great hope was that code review would be solely a preventive activity: do code review before execution, and there won’t be anything to debug. That’s not going to happen. Too much code is delivered under pressure, without code review. But all is not lost. Just review the code when debugging. Look again at the three steps — how does code review, as defined above, change debugging? First, don’t debug alone. Whatever you may think of pair programming for or against, pair debugging is a must. Especially if you wrote the code, having someone else look for the problem, or better, restate the problem, is essential. Second — discussion. Debugging that rushes to a fix often produces the wrong fix. And finally, effective writing. Sure there’s the writing which is fixing the code. But what about adding comments? And perhaps more important and effective, especially if it’s a quick fix: submit a new defect or change request to your tracking system for what really needs changing to reduce the chance of this failure recurring.

Static Analysis

Tools! If I have tools that can find problems, I won’t have to review code. Yes, you will. And you should want to. Static Analysis tools are great for finding simple errors, and potential defects. For narrowing the field of focus. But then, to find out what’s really wrong … code review. Additional benefit of the tool — since it has no feelings, it can be two of you against the reviewer: read the code with a partner, discuss why the tool thinks there’s a problem and whether you agree, and again, effective writing. Not just the fix, but why that fix, and also perhaps an adjustment to the tool so it will do better next time. High-end static analysis systems let you document that right inside the tool.

Testing

Testing, and its partner, code coverage, are often seen as the opposite of code review. Whatever I couldn’t catch by code review (and static analysis) is left for exercising the code under controlled conditions, causing failures, and fixing them. Actually code review is the most important part of developer testing. Not only reviewing the code under test in order to design good white-box tests. But also reviewing the code based on the test results and coverage results. Why does the software intermittently fail this test? Why is it so difficult to cover all the branches in this function? Again, read together, discuss, write. Here the effective writing is likely to be refactoring the code. If someone wants code review “documentation” — give them a diff and a one-sentence explanation of what you did and why.

Code Review is All-Purpose

There are many more activities in the developer’s daily life. Detailed design, branching and merging, delivery — all can benefit from in-line code review with this three-step model of reading, discussion, and effective results-writing. And by acknowledging this model, we see that code review, or almost code review, is happening all the time. Just have to complete the steps to get code review that real people will do.

As good as the misconceptions surrounding Toyota make it sound, the truth is even better.

– Stewart Anderson in Quality Digest’s Quality Insider 20-Oct-2009

Thanks to Stewart Anderson, both for bringing us the real story about how Toyota, famed for focus on quality, actually goes about doing it. Put aside for a moment what you know about ISO, QMS, metrics etc and read the carefully crafted wisdom of experience. How quality really works at Toyota. And yes, you really can apply these ideas elsewhere, even in software development.

All quotes are from the linked article by Stewart Anderson.

Toyota’s basic pattern for improving a process is based on a simple three-part model:

1. Understanding the current condition.
2. Developing and defining a target condition.
3. Understanding and tackling problems which need to be overcome to move from the current condition to the target condition.

This model has learning at its heart [to identify] actions to solve problems in the current condition.

The primary responsibility [of the team leader] is to monitor the process, ensure that standard work is being followed, and coach and mentor the work team in improving the process. Team leaders receive special training in process improvement and problem solving ….

To read the full article click here.

Related reading: Gemba Kaizen: A Commonsense, Low-Cost Approach to Management (my review here).

Full disclosure: My Toyota is over 15 years old and still running fine. My last car was also a Toyota.

Someone comes to me with a problem that sounds like,

“I wanted to do X but it doesn’t work.”

When I hear that request, often they know a lot more about the technical details than I do. I need them to teach me in a way that lets me put things in order. My method starts from success … and progresses to failure. Strange? But it works.

The questioner is focused on the failure scenario. I crave success. There must be some around — just have to find it. I ask what similar scenario succeeds. What version of the file did work? What build compiled successfully? When did this test last pass? As we discuss the successful scenario, I learn why it worked. What requirements were met by that configuration. I repeat those requirements out loud for confirmation.

With a success in hand, well understood, we have a checklist to apply to the failed configuration. We can go over the failure and check whether all the things that made something similar work are present. Of course, one will be missing. We have found the problem. I may not know how to fix it. But usually the questioner does.

A new project is ramping up and needs more resources. Another project is winding down — maybe it can do with less.

Let’s move some people from one team to the other

What’s wrong with this common resource allocation decision? It’s not the decision. Moving resources to the project that needs them makes sense. What’s wrong is the words, which hide a false assumption.

Here, “team” means location. Move people from one place to another. Like conductors putting people on a train: the “teams” are two train stations. One called “Project A Station” and the other one, “Project B Station”.

But teams are not train stations. A team is not the room it sits in, or its box on the org chart. A team is the particular combination of people — their skills and personalities — and their shared experience. They know their project or product best right now, and equally important, they know each other and the special role each person fills. Reassigning people from one team to another is more than moving a person. It is actually dismantling the first team, and disturbing the second. The movement of people based on the train-station model of teams leads to a contradiction: both teams — origin and destination — cease to exist.

If we must model people and treat them as resources, a better model is a hand at cards. The managers are the players and partners; the game is getting more projects completed. There’s a fixed deck — the “human resources” of the company. Some cards may be similar in strength or level, but every card is unique.

The value of your hand is based entirely on the combination of cards you’re holding right now. If you or your partners need a stronger hand, everyone knows that some cards will have to be exchanged. Put down 2, pick up 2. I pick up what you put down. Resources. Cards. I know this doesn’t sound any better to workers than “counting heads”, but it is. Because every card player thinks twice before putting down a good card, and prefers to take over an entire good hand if they can.

Recently I found an excuse to dust off some old lab books of introductory science experiments. They were the curriculum for a 9th grade Physical Science I class at my high school; I wrote them as a summer job during college. I didn’t invent the experiments. Rather, the head teacher gave me outlines, and my job was to try them out and make sure they would work, so that prospective students wouldn’t get frustrated and turned off by science.

The excuse was a friend, a scientist, who is meeting a public service requirement of her studies by volunteering to teach science to schoolchildren, and to their teachers. I offered my science course as possible material for her classwork. As we looked over the experiments, we doubted a bit how even 9th graders might learn from them, or be as excited about them as I was when I built and tested them myself. Sure, the projects were easy, and they had worked. But would the experience of building these projects — static electricity demonstrations, electromagnets, motors — be enough to convey the principles behind their operation? Would the kids see the point?

Yesterday I was in the kitchen preparing a simple lunch of tacos and beans. Washing and cutting lettuce, chopping an onion, peeling a cucumber, grating cheese, warming the beans with some tomato sauce. In the quiet afternoon, I couldn’t help noticing the crunch of the knife through the lettuce. Or wondering how best to preserve the other half of the cucumber, now leaking water from its open end. And why do we grate cheese? So much science, here in the kitchen!

Now with the internet, you can search “science in the kitchen” and get pages and pages of websites with all sorts of neat science projects in the kitchen. Let alone YouTube with some pretty exciting and dangerous experiments. (Be warned!) I have no doubt that school science textbooks have taken this to heart and now include experiments that relate to the real world. And yet, science enrollment declines.

Even with the best of intentions, popular science experiments in the kitchen won’t do it. Science is not in school, nor is it in the kitchen. Science is not an activity, but a way (just one way) of looking at the world and making sense of it. What excited me in science class was not the sitting and listening to the lecture. It was making the connections with daily life outside class, and enjoying the beauty of the natural world with new understanding.

Those understandings could come from things as simple as basic cell structure in biology. There’s lots of water in a living cell, so when you peel and cut a cucumber, it gets wet. Or as complex as thermodynamics. One college winter, I learned that there’s really never a flow of “cold”, but only heat transfer. For a good month after that, climbing the steps to class, I would grasp the banister outdoors, and instead of feeling cold, I felt the heat flowing out of my hand into the metal. These experiences are what brought me back to study even more.

Science teachers have to have and share that excitement. If they’re not science experts themselves, no matter. They are learning adults, who can build their own understanding and catch the excitement from science mentors. The key has to be in giving the right answer to the question so many students ask: “But what is this good for?” The wrong answers are the allegedly practical ones — advancing technology, getting a job, or passing the test. The right answer, the one that should guide science teaching, and bring the kids back to class, is “Because the world around you is beautiful, and science gives you eyes to see it.”

It’s pretty well known that there are three ways to improve software quality: improve process, improve methods, and improve tools. But these are the generic concepts — applicable to any software development effort. Today’s topic is more specific and closer to home. Product quality.

Isn’t it obvious? The software product is the running software, and product quality is how well it meets its requirements. Simply put — does it do what people expect it to do, and not crash or lose data?

But organizations which release software focusing only on that one aspect of product quality are often unpleasantly surprised by inconsistency. They are doing all the right things — following a good process, using good methods and tools, and regularly reviewing all those to see what can be improved. Yet sometimes the product is better, sometimes worse. Why?

External product quality — in software, that’s how well the software serves its users when it’s running, is not enough. There’s also internal quality. The readability of the codebase. Code that’s well-written — easy to understand quickly — is easier to maintain. Fewer programmer errors, and fewer schedule slips. We can add to that clear and organized debug output, which avoids false positives, and gives an accurate record of when the running software seemed to be fine, but was really operating on the edge — just barely recovering from failures.

But there’s another part of the product that’s often overlooked. Overlooked when reorganizing teams, or just when parceling out the daily assignments from multiple projects. The developers. If you take a single iteration of a software product, with development at its best, you’ll apply best process, methods, and tools to put out the next version of the software, and maybe even have codebase improvement (cleanup, refactoring) on the list too. Get to release and you deliver good running software, more maintainable code and … the new state of the team that developed it.

The team members may have learned new methods or technologies. Or just how to avoid pitfalls from the past. Equally important, that particular team learned how to work together — not in general, but specifically in today’s process, methods & tools, and codebase. And with each other. This third part of the product cannot be ignored or thrown away, just as nobody would think to delete a code branch delivered to the customer, or switch out a working library in mid-project.

Looking further at this third component of the product — the team that delivered it — we see clearly that those quality improvement concepts of process, methods, and tools do not exist in a vacuum. They are not just abstract ideas. Rather, they exist in the team members who develop a product, and are unified by teamwork — which means this team learning to work together with these people on this product.

So when you think of product quality improvement, plan to improve all of these:

1. How the product works
2. How the codebase looks
3. How the team works

And at the end of an iteration, make sure to review and protect all three. That’s the guarantee that the next version of the product will be even better.

That’s right, not “development” but “developer”.

The latest issue of The Embedded Muse newsletter—#179 (pdf)—has a reference to an interesting and very different kind of book about C. It’s about the business pressures and the thought processes of C software developers. And a lot more. It’s also long—1600 pages! So download a copy and skim for what interests you:

The New C Standard: An Economic and Cultural Commentary

Some lines that caught my eye, just in the first 100 pages, skimming with the “Page Down” key:

“Software developer interaction with source code occurs over a variety of timescales [for example, those] whose timescale are measured in seconds.”

“The act of reading and writing software has an immediate personal cost.”

“Source code faults are nearly always clichés; that is, developers tend to repeat the mistakes of others and their own previous mistakes.”

“A list of possible deviations should be an integral part of any coding guideline.”

“The following are different ways of reading source code, as it might be applied during code reviews …”

Other related references from the book’s author Derek M. Jones (http://www.knosof.co.uk/):

• Blog: http://shape-of-code.coding-guidelines.com/
• Article:  Coding Guidelines: Fact and Fiction
• Article: The 7±2 Urban Legend (small pdf)

And somehow related, from another author, Les Hatton (http://www.leshatton.org/):

• Article: Testing the value of checklists in code inspections

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.

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.