Solutions

There are many ways to fix this problem.

We could revert the header file and curse the engineer who violated the fundamental rule about fixing things which ain't broke. This approach, though, risks destabilising any newer code which assumes a Signal is unsigned.

We could recast the arithmetic so it works independently of signedness:

    bool
    signalsDifferent(Signal s1, Signal s2, int tol)
    {
        return 
            s1 > s2 + tol || s2 > s1 + tol;
    }

Note, though, that even this version of the function may catch us out if the additions overflow.

Or we could lean on the type system and get it working for us. The driver layer is written in C and may well prefer to offer the middleware built-in types but that doesn't mean the C++ middleware shouldn't use something safer: a range checked integer class (such as the one by Hubert Matthews [1]) perhaps:

    typedef CheckedInt<0, 100, SaturatedArithmetic> SignalPercent;

The perfect solution will have to be left to the reader. I'm not sure there is one. Finding the problem was the hard part: we really want to avoid similar problems in future.

Creating more specialised classes is no panacea. Such classes often end up implementing cast operators for ease-of-use which can make them susceptible to the very problems they're meant to prevent—only now we have an extra layer of abstraction to wade through. Matthews' implementation is sound, and the accompanying text describes the subtleties involved in getting such an apparently simple class right. Unfortunately it does not work out of the box on the Talk-To-Me, whose C++ compiler is closer to the embedded C++ specification than ISO/IEC 14882: 2003[2]).

Lessons Learned

There are several lessons we can learn from this.

Invest in unit tests. They catch bugs in even the simplest code.
Find a way to simulate poor signal conditions back in the lab, rather than waiting for field tests to throw up this kind of problem. This principle generalises: make sure your system tests simulate a full range of operating conditions.
If the types used in an interface must change in this way, don't make the changes backwards compatible. So, typedef int Signal would migrate to typedef unsigned SignalPercent rather than change silently. This means client code has to at least notice the change.
Take special care with the built-in types. They are designed to provide the most efficient code you can get on a platform short of writing machine code and, as such, are inevitably a source of portability problems. Although the standard lays down what you can expect, what you can expect often comes down to "implementation defined". When will an int overflow? What happens when you right shift a negative signed value? Is a char signed or unsigned? (Earlier, when I said that 10u - 20 was a very big unsigned number I wasn't being vague: the actual value is platform dependent.)

Defect Two

The Problem

Despite its tiny screen, the Talk-To-Me provides a rather neat email client. Users can navigate up and down a list of emails or view the selected email full-screen.

Once again, system testing indicates a problem.

The full-screen email view doesn't make use of the full screen. Text is being wrapped a few pixels short of the right hand boundary. This wouldn't matter much for a PC-based email client, which typically grabs plenty of space for toolbars, sliders and so forth—but on the Talk-To-Me's 288 by 256 pixel display, this is a big shame.

The Cause

The Talk-To-Me engineers are less suspicious of the driver layer this time. Other parts of the user interface use the full extent of the screen just fine.

Once investigated, the problem is swiftly tracked down to the following piece of code:

    /**
     * Render email full screen
     */
    void Email::fullScreen() const
    {
         Rectangle const 
             full_screen(0, 0, 288, 256);
         
         textRender(text,
                    full_screen,
                    true); // wrap text
    }

Here, textRender() requires as input:

the text data to render
the display region (a reference to a constant Rectangle)
a flag (which defaults to false) indicating whether or not the text should be wrapped.



Analysis

The numeric literals 288, 256 are not pretty but the Talk-To-Me comes with 
no other display options.  The problem here is that the 
Rectangle constructor wanted them the other way round.  

    class Rectangle
    {
    public: 
        /**
         * Construct a rectangle with the supplied input dimensions.
         */
         Rectangle(int top,
                   int left,
                   int bottom,
                   int right);

     ....

     };


So, the email was rendered to a width of 256 rather than 288 pixels.


The programmer makes the fix, feels ashamed (this has happened before), but
won't get fooled again (fingers crossed).

Solutions

It's rather difficult to see how to guard against this sort of problem.  
The Rectangle class is fine; the Email class is 
using it in a reasonable way.  


A unit test would have to be cunning.  A system tester has to watch
carefully.


You could argue—and I would agree—that the full screen rectangle ought
to be defined just once and passed around as needed. That should sort out this
particular problem and maybe a few similar ones. However most rectangles
aren't full screen ones, so the problem hasn't been eradicated.


You could also argue that the type system could again be used to help—isn't
it a type error to pass parameters in the wrong order?—but it's hard
to see how X and Y coordinates could sensibly be made different types.


You can name your inputs:
    int const top    = 0;
    int const left   = 0;
    int const bottom = 256;
    int const right  = 288;

    Rectangle const 
        full_screen(top, left, bottom, right);


or resort to comments:

    Rectangle const 
        full_screen(0,    // top  
                    0,    // left
                    256,  // bottom
                    288); // right


These solutions are fragile. The compiler doesn't check comments for accuracy
nor will it spot if your nicely named parameters match those declared in the
constructor.




<<  Page 2 of 4  >>






The C++ Source | 
C++ Community News | 
Discuss | 
Print | 
Email | 
First Page | 
Previous | 
Next


	Web Artima.com