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.

One of the big human challenges to code review is ego. For all the talk about “egoless programming”, ego still means “self” and when a person writes code, or owns it for maintenance, s/he identifies with the code. That kind of “ego” or “self” thinking is what lets the code author work through problems by saying, “let’s see, when I receive this signal, I enter this loop and I calculate that value.” So how can code author and code reviewer(s) go into a code review, whether it be a meeting or online, and yet avoid criticism and defensiveness?

Here’s an image that can help: when code review participants enter the review, imagine everyone sitting on the same side of the table, with only the code on the other side. The code author is no longer the author or owner — just another reviewer. The two or more people in the room, or in the online review, form a team together to find as many things as possible in the code to improve. Now nobody needs to defend the code, because everyone is a reviewer. Still, though, if criticism is in the air, the real code author will feel it.

The desired result of a code review is a list of important things to improve. Comments which the code author can take back to the code and say, “I understand what I need to do to make this code better.” Take a concept from the topic of assertive communication: The “I” message. Best explained by an example:

Let’s say I’m a busy, pressured group leader who has to lead meetings, and someone is always late for the group meetings — much later than everyone else . I decide to speak to that person. What can I say to him that will cause him to feel what I feel, and based on that, to join me in solving the problem? Not to feel criticized, and to become defensive. An “I” message would be:

“I’m under a lot of pressure when leading our weekly status meetings, and when I see you come in 20 minutes late to the meeting, it makes me more tense, and makes it hard for me to keep the meeting on track.”

The idea is that, while I have mentioned the person’s lateness, I have described only what I see, and described a problem that I have. All the focus is on me and my problem, which the group member may realize is his problem too, since he may benefit from a calmer group leader and a more effective meeting. Hopefully, he is drawn into solving my, or our, problem.

Similarly with code review. Instead of a critical comment like, “this code is very confusing,” say, “when I read through this function — both the comments and the code — I get to the end and I still have no idea what it’s really supposed to do. If I had a change request on this code, I would not be able to confidently make the change.” Given that the code author probably does not have the same problem, this comment draws the author into saying, “I understand what the function is supposed to do, but apparently you don’t from reading the code. I’ll try to improve the comments or clarify the code so that you understand what I understand.”

Try this method in your code reviews to get everyone working together against the code, to improve the code.

We may never know all the details of the recent Toyota failures, because, unlike the Challenger and NASA, Toyota is a commercial entity rather than taxpayer-funded government. And it’s a bit early to be looking for definitive analysis. What we can do, as embedded software developers, is realize what we’re up against, and how it didn’t start yesterday.

When microprocessors first came out the buzz on the news was that soon they were going to be inside everything. I remember wondering what do we need them in everything for? My washer and my microwave oven, for example, worked just fine with a mechanical dial. But I was wrong and now they are in everything.

Much earlier on, complex systems were based on the Six Simple Machines. I can’t say which is simplest, but maybe it’s the lever. What’s important here is that the behavior of the lever is governed by physical properties of solids, which by now are pretty well understood. Push down on one end, and the other end goes up. Exceptions exist: solids can stretch under tension, deform under pressure, and if you push too hard, they can break. Apparently Toyota had problems with this too, in their gas pedal problem and fix.

Technical advances brought hydraulics — based on the properties of fluids and gases. I also remember replacing a brake line on an old car once, and getting to see how a leak in the line leads to spongy braking, and, if I hadn’t been refilling it weekly before the replacement, to brake failure.

In both of those technologies, the basic idea is that the system user (e.g. the driver) pushes on the control, force is transmitted immediately through a physical medium, and the desired effect occurs. Failure modes are well understood and (usually) are prevented.

Now replace the lever with software. No more physical properties: the behavior of the software transmission line is governed by the behavior of people — software developers writing lists of instructions which are translated several times before reaching the hardware. Now you push on the control, and a cacophony of instructions (okay, I’m being unfair — if the software is well-written, it’s a concert of instructions) rush down a wire to an actuating motor at the other end.

Let’s be clear about this: real-time software is a myth. The metal of the lever, or even the fluid of the hydraulic line, has been removed and replaced by your entire software development organization. Materials science out the door, organizational sociology in its place. If you want to know where that leaves us, read this short post about drive-by-wire, and the many comments there.

What’s the answer? In the code itself — perhaps it’s avoiding interrupts and multi-threading, and moving to synchronous programming where everything proceeds in lock-step. At least then a modern car would run like clockwork. Other than that, notice that many of the suggestions in the discussion talk about mechanical back-up systems, which are basically putting the lever back into the system.

But software development is not, as commonly believed, about technology. It is about writing, reading, and communication in ever-larger groups. Those who have said, “The pen is mightier than the sword” were thinking of politics. Now writing, in the form of software, interposes itself between every hand and hammer. We have broken the lever; do we know how to wield its replacement?

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.

What kind of tool is required for code review? What are we trying to accomplish? How much of the code must be reviewed at a time, and why? These are some of the questions that we get caught up in, that prevent us from deploying effective code review in a development organization. One way to cut through the arguments and move forward is to realize that there are different types of code review, with different goals. Here are some parameters of two major, but different types:

Code Review type 1

Key question: Does this change accomplish what it was supposed to, while doing no damage?
Desired code author response to comments: Revise changes
Under review: changed files with diff
Tool: diff comparer
Required feature as a code review tool: right-click submit comment
Nice-to-have features (but could be in a separate app): right-click definitions

Code Review type 2

Key Question: Is this code designed well, so that it is easy to fix and extend?
Desired code author response to comments: Refactoring / renaming
Under review: any subset of entire codebase, starting from a given area of the code
Tool: source code browser
Required feature as a code review tool: right-click submit comment
Nice-to-have features (but could be in a separate app): code editing

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.