| One
of the easiest ways to start an argument among development managers is
to mention refactoring. Martin Fowler describes refactoring as
"the process of changing a software
system in such a way that it does not alter the external behavior of the
code yet improves its internal structure. It is a disciplined way
to clean up code that minimizes the chances of introducing bugs.
In essence when you refactor you are improving the design of the code
after it has been written."
- Refactoring, preface.
Of course, some managers regard this as
anathema. If it ain't broke, don't fix it. On the other
hand, an overly complex design will sooner or later bite you. We've
all heard of systems that are so complex that they cannot be modified--lest
something break--or that are extraordinarily difficult (and expensive)
to debug. Some of us have even had the misfortune of working on
such a system. Refactoring helps prevent a system from reaching
an expensive, unmaintainable state.
Refactoring is also counterintuitive because,
according to traditional software processes, design precedes coding.
When we refactor, of course, we are redesigning after we see the code.
The reason this makes sense is that not even the best programmer makes
the best design decision the first time, every time. Thus every
software system contains the seeds of its own downfall--today's overly
complex but otherwise "correct" design can become tomorrow's
maintenance nightmare. I would compare refactoring code to weeding
a garden. When the weeds are small the tomatoes can grow today;
leave the weeds undisturbed, though, and eventually they will choke your
garden.
Recently I reviewed some VB.NET code written
by a team new to object-oriented development. While they handled
many tasks well, they had no experience with simplifying a complex class
structure. Numerous switch statements were threatening to
choke their codebase. A switch statement is the way that procedural
code handles multiple logical conditions. In object-oriented code,
though, we should try to simplify our code by using polymorphism to deal
with the varying conditions.
Let's examine some C# code that loosely
resembles my client's original code. The class bbomTheme (BBOM ==
"Big Ball of Mud") is responsible for exposing properties that
can be used to provide a visual theme for an ASP.NET application.
Since a client can choose between a variety of themes, some class must
take responsibility for distinguishing between the themes and ensuring
that the correct attribute is exposed by each property. Class
bbomTheme handles this responsibility by itself, first by setting private
member _strThemeName in the constructor:
public
bbomTheme(string
strThemeName) {
_strThemeName = strThemeName;
}
Then each property uses _strThemeName
in a switch statement to determine what value to return. The PanelColor
property provides an example of this:
public
Color PanelColor
{
get
{
switch(_strThemeName)
{
case "Excitement":
return Color.Red;
case "Joy":
return Color.Yellow;
case "Blues":
return Color.Blue;
case "Regal":
return Color.Purple;
default:
return Color.White;
} //end
switch
} //end get
} //end property
Since class bbomTheme implements
seven properties, it contains seven switch statements. The responsibility
for ensuring the correct theme is used has been distributed broadly through
the class. An experienced designer will recognize that this is not
good. Because the responsibility for exposing correct attributes
has been broadly distributed, modifying the code involves a lot of little
changes in a lot of different places. What happens if we need to
add a new theme? We will have to modify seven switch statements.
And what if we need to add a new property? We will have to write
a new switch statement, which increases the complexity of our class.
This is how code becomes unmanageable. The irony is that
the UML class diagram for bbomTheme and the page that uses it does not
seem complex (see diagram 1).

Diagram 1. Class diagram
If we believe that there will never be
any need to modify the behavior or attributes of the class, there is no
reason to refactor the code. After all, the problem of difficult
maintenance vanishes if we don't have to perform any maintenance.
However, the team I was working with was confident that a customer would
want to employ a new visual theme. The quick-and-dirty strategy
to accomodate this request is to add a new case to the switch statement
in each property. This will be an error-prone process, as we will
need to verify that we don't miss a property, which would cause its "getter"
to fall through to the default at runtime. Further, this error would
be virtually impossible to detect by inspecting the code, and could easily
be missed by a tester. Modifying the code is thus difficult and
expensive, and will make our code even harder to maintain and extend,
to boot. As Steve McConnell notes in Software Project Survival
Guide, "The problem with quick and dirty...is that dirty remains
long after quick has been forgotten."
A better approach is to refactor the code
to make it simpler. We can be pretty certain that we will be asked
to modify this code yet again in the future; if a customer is asking for
a new theme now, it is likely that at least one customer will want a new
theme in the future as well. So while refactoring may or may not
reduce the cost of this modification, it will definitely reduce
our long-term costs.
When we refactor code, we look for a design
pattern that will simplify it. In this case, after reviewing the
candidate patterns we decided to use the parameterized factory design
pattern. The parameterized factory pattern assigns responsibility
for creating the right object to a method in a factory class. (See
Erich Gamma, et al., Design Patterns, pp. 110 - 111.) The
right object will be one of several objects that implement an interface,
in this case the ITheme interface. Thus all the knowledge necessary
for choosing the right theme will be encapsulated in one small switch
statement in the factory's GetTheme method.
Let's examine how this makes our code simpler.
The factory class' code is straightforward:
public class
ThemeFactory
{
public
static ITheme
GetTheme(string
themeName)
{
switch(themeName)
{
case "Excitement":
return new
ExcitementTheme();
case "Joy":
return new
JoyTheme();
case "Blues":
return new
BluesTheme();
case "Regal":
return new
RegalTheme();
default:
// we should never reach this default from properly written client code.
// However, we only emit debug information in debug builds; in release
builds,
// we return a default theme so that the user can get his/her work done.
Debug.Fail("The theme specified is not known by the ThemeFactory constructor");
return new
ExcitementTheme();
} //end
switch
} //end method
} //end class
ThemeFactory.GetTheme() contains
the only switch statement in our code. It is declared as static
because there is no need to go through the overhead of class instantiation
when the ThemeFactory class has no instance variables. Having created
the ThemeFactory class, if a customer wants to add a new theme, we will
only need to add a single case, just two lines of code, to our switch
statement. (We will also require a new class to implement the ITheme
interface, but we'll get to that in a minute.) Also note that we
have improved our code by adding a call to Debug.Fail() in the default
case. Should client ASP.NET code specify a theme for
which we don't have a corresponding class, a debug build of our code emits
debug information. This is not something that our test team would
miss (and heaven help us if they did!). On the other hand, a release
build will omit the call to Debug.Fail and simply return a default theme,
which will allow a user to continue to get work done. Note that
to accomplish this before the refactoring, we would have had to add the
Debug.Fail call in seven different places. (And then if we had changed
the strategy for handling defaults again, we would have had to modify
the code in seven different places once more. This kind of situation
makes us hesitate before changing our code at all, regardless of how much
better the class' behavior might become.) So by refactoring and
simplifying our code, we have made it possible not only to address client-requested
modifications more quickly, accurately, and inexpensively, but also to
improve the resilience of our code with ease.
Second, we have to define the ITheme interface
and implement it in various subclasses. The code is simplicity itself:
public
interface ITheme
{
Color PanelColor
{
get;
}
Color BackColor
{
get;
}
// other properties omitted for
brevity's sake...
}
public
class ExcitementTheme:
ITheme
{
public
Color PanelColor
{
get
{
return Color.Red;
}
}
public
Color BackColor
{
get
{
return Color.Blue;
}
}
// other properties omitted for
brevity's sake...
} // end class
Now if we forget to implement a
property in a new Theme class, the compiler will inform us before our
code can ever reach the test team. Also, all the properties of the
ITheme-implementing class written for our customer will be isolated in
a single location, rather than being dispersed throughout many different
switch statements. This makes it easier to verify correctness via
code inspection, and makes it much easier to port customized code
from this version to the next version of our system.
Finally, we must modify our client code
to use the ITheme interface. This is very simple:
private
void ListBox1_SelectedIndexChanged(object
sender, System.EventArgs e)
{
ITheme myTheme = ThemeFactory.GetTheme(ListBox1.SelectedItem.Text);
Panel1.BackColor = myTheme.PanelColor;
lblHello.BackColor = myTheme.BackColor;
lblHello.ForeColor = myTheme.ForeColor;
lblHello.BorderStyle = myTheme.Style;
lblHello.Font.Name = myTheme.Font;
lblHello.Font.Bold = myTheme.Bolded;
lblHello.Font.Italic = myTheme.Italicized;
}
By changing the type of myTheme
from bbomTheme to ITheme, we only had to change a single line of code.
Everything else remained the same.

Diagram 2. Class diagram of parameterized
factory pattern
The irony of refactoring is that our class
diagram has become a little busier (see Diagram 2). This will always
be the case, though, when we isolate responsibilities into separate classes,
rather than stuffing them into a single "DoEverything" class.
A close examination of the new diagram reveals that the classes have well-defined
responsibilities and interfaces, in contrast to the earlier diagram.
While the number of classes is greater in our new diagram, our interface-based
solution hides the details of the classes that implement ITheme from the
client code.
What can we conclude from this little exercise?
* Beware of assigning too many responsibilities
to a single class.
* The presence of many switch statements
in object-oriented code signals an opportunity to simplify by using polymorphism.
* When modifying code, be ready to refactor
to make it simpler and less expensive to maintain.
* When refactoring, evaluate candidate
design patterns, then choose the pattern that best simplifies your code.
* The benefits of object-oriented programming
often result from using interface-based programming, rather than implementation
inheritance.
* Consider using the parameterized factory
design pattern when your code needs to choose among several implementations
of an interface, based on user selection or configuration files.
* Use Debug.Assert() and Debug.Fail() statements in your code to help
you discover errors in debug builds, but respond graciously to client
requests in release builds.
Download
the Code that accompanies this article
An independent consultant based in Charlotte, NC, Chris has spent
over 8 years in software development, including 5 years as an Escalation
Engineer and Application Development Consultant at Microsoft.
He is also the principal of his family's home school. You can contact
him at chrisfalter@hotmail.com.
|