Is Your Code SOLID? OCP and Fighting the Cost Of Change

Posted by Jason Smith - 03/12/09 at 05:12:41 pm

It is better to keep your mouth closed and let people think you are a fool than to open it and remove all doubt. – Depending on who you ask, Mark Twain or Abraham Lincoln

Is it a good use of time and money to periodically pull reviewed, tested and unchanged code out of source control so the team can re-review and re-test it?

Probably not. If the code hasn’t changed, we’ll quickly experience diminishing returns.

Then again, all software changes, so it’s a moot point.

Or is it?

What if we could structure our designs in such a way that we can add features, without changing existing code?

Enter the Open/Closed Principle (OCP): software should be open to extension, but closed for modification.

To understand how this principle works, lets start with code that violates this principle. We usually find this around dynamic casts, “switch/case” and chains of if/else if/else statements.

Consider the switch/case statement. Every time we add a new case, we must modify code in the switch. Same with if/else: every time we need a new else, the original source must be modified. If behavior in a function is dependent on a successful dynamic cast of a parameter (using the as or is keywords), you’ll have to add new code to handle any new types. All this new code called in all these places must be tested in all its variations.

Here’s an example:

public class Foo
{
      private object myStuff;

      public void Bar(OutputDevice outputDevice)
      {
         switch(outputDevice)
         {
            case OutputDevice.Window:
                 DrawOnWindow(myStuff);
                 break;

            case OutputDevice.Printer:
                 PrintStuff(myStuff);
                 break;

            default:
                 throw new NotImplementedException();
         }
     }
 }

Assume we get a new feature request: users want to save their stuff to a file. With the code as it’s currently written, we must modify the original shipping code and add an OutputDevice.File case (and quite possibly a new enumeration value – potientially breaking any other code using the original enumeration). Additionally, we need to test Foo.Bar to make sure our changes didn’t break the old code, and because Foo.Bar changed, we need to test all the code that calls Foo.Bar, and we need to test the new rendering codes to make sure that works.

Way, way, too much work for a “simple” change.

An OCP solution for this will turn all these conditionals into abstractions:

public interface IRenderStuff
{
    void Render(object data);
}
public class RenderableWindow : IRenderStuff
{
    public void Render(object data)
    {
        // ...
    }
}
public class PrintRenderer: IRenderStuff
{
    public Printer Printer { get; set; }
    public void Render(object data)
    {
        Printer.Setup();
        Printer.Print(data.ToString());
        Printer.Close();
    }
}
public class PrintRenderer : IRenderStuff
{
    public void Render(object data)
    {
        using (System.IO.TextWriter writer = CreateFileWriter())
        {
            writer.Write(data.ToString());
        }
    }
}

Now, Foo.Bar can look like:

public void Bar(IRenderStuff renderer)
{
     renderer.Render(myStuff);
}

Foo.Bar is now open for extension, and closed for modification. This one adjustment insures Foo.Bar’s doesn’t have to change when we add a new renderer. If it doesn’t change, it won’t have a bug injected into it (if the only change is a new rendering destination), and no longer needs to go through testing of all the rendering cases.

In fact, Foo.Bar need only be tested against a one stub implementation of IRenderStuff to verify it is holding up its end of the contract. Of course, each IRenderStuff implementation needs testing, but we had to do that anyway.

Here’s the payoff: this change reduced our test overhead (read: ways this code could break) from

NumberOfRenderedObjects
×
NumberOfRenderingMethods
×
NumberOfBodiesOfCodeUsingTheOutputDeviceSwitch

to

NumberOfObjectsRendered + NumberOfRenderingMethods

Our cost of change just went from an exponential increase to a linear one.

Now, consider class member variables. Whenever code reaches into a class to access a member directly, we cannot change how that class is implemented without affecting (and potentially breaking) all of that class’ users. Every bit of code referencing a member variable is open (or “exposed”) to modification. Any change to our implementation means a change to all of its consumers.

To limit this effect, we encapsulate the variables behind methods and properties, closing those relationships to modifications in the code.

It’s not much of a leap to realize any member variable access violates this principle. So we minimize the area affected by making member variables private (not protected, that’s a cheap cop-out). By doing this, only member functions of the defining class are exposed to changes in those variables. All other consumers are closed to those changes.

The same thing applies to global variables. In the same way non private data members violate the OCP, so do global variables. Every module that depends on a global variable can never be closed to modification with respect to that global.

Powered by WordPress with GimpStyle Theme design by Horacio Bella.
Entries and comments feeds. Valid XHTML and CSS.