This post originated from an RSS feed registered with Agile Buzz by Keith Ray. Original Post: Keeping the Design Good Feed Title: MemoRanda Feed URL: http://homepage.mac.com/1/homepage404ErrorPage.html Feed Description: Keith Ray's notes to be remembered on agile software development, project management, oo programming, and other topics. Latest Agile Buzz Posts Latest Agile Buzz Posts by Keith Ray Latest Posts From MemoRanda

It's long been accepted that software inevitably degrades until it has to be re-written. Advocates of refactoring and iterative/incremental design/implementation reject that idea. Software doesn't degrade unless acted upon by an outside force (most often, programmers).

Here's an example of a new requirement, and two ways to satisfy that requirement. One way creates an unsafe "lump" of bad design. The other way modifies the existing design so that the solution-code for the new requirement continues to meet the criteria for good design.

The new requirement, for Apple MacOS X developers using Metrowerks PowerPlant, is Intel support/Universal Binaries. Specifically, the PowerPlant resource files that are in PowerPC (Motorola byte-order) need to work on Intel (Intel byte-order) as well as PowerPC, preferably without modification. These resources are called Ppob resources, and the are stored in binary form.

Apple provided a lump of code that flips the bytes in Ppob resources here. Notice how the function _FlipPPob depends on the layout of almost every PowerPlant view/control class. Notice the big switch statement. Note the tons of duplicated code, such as calls to FlipViewData.

One unsafe aspect of this _FlipPPob function is that it attempts to work with every view/control class, but there's a danger in that it doesn't really work with every view/control class. The author of this code may have left some view/control classes out, and of course programmers can (and do) define their own subclasses of PowerPlant view/control classes.

Let's re-examine the existing design. Ppob resource reading is invoked by UReanimator. It reads enough to identify a view/control class, and creates an instance of that class via a set of nifty templatized factory objects. By using the factory objects, the UReanimator class does not depend on any concrete view/control classes. To read the PPob resource, UReanimator creates an instance of LDataStream, that it passes into the view/control object, which reads the Ppob data from that stream (typed as LStream, the abstract parent class of LDataStream). Each view/control class knows how to read its own data layout from the stream.

While we could modify every view/control class to byte-swap the data it reads from the LDataStream, that would be duplicated code. The place to put the byte-swapping is in LDataStream, LStream, or a byte-swapping subclass of LDataStream or LStream. If we are running on an Intel platform, UReanimator can instantiate the new byte-swapping LStream-derived class (or pass in a parameter to LDataStream) The method that reads an int can be transformed from something like this:

LStream & LStream::operator >> (UInt32 & outNum)
{
    ReadBlock(& outNum, sizeof(outNum));
    return (*this);
}

to something like this:

LStream & LStream::operator >> (UInt32 & outNum)
{
    ReadBlock(& outNum, sizeof(outNum));
    if ( mByteSwapping )
    {
        outNum = ByteSwapUInt32( outNum );
    }
    return (*this);
}

Instead of a lump of poorly designed code (780 lines worth), we have a change in the UReanimator class (probably a one-line change) and a few changes to a existing LStream class; probably less than 100 lines of new code. The new changes are localized. They are not dependent on all the existing PowerPlant view/control classes. Any view/control class that we didn't plan for will still work. We have modified the code to handle a new requirement, without degrading the design.