Previous post in this series:
SOLID – SRP – Ignoring the rules on the paint can

SOFTWARE ENTITIES (CLASSES, MODULES, FUNCTIONS, ETC.) SHOULD BE OPEN FOR EXTENSION, BUT CLOSED FOR MODIFICATION.

When a single change to a program results in a cascade of changes to dependent modules, that program exhibits the undesirable attributes that we have to come to associate with “bad” design. The program becomes fragile, rigid, unpredictable and unreusable. The open-closed principle attacks this in a very straightforward way. It says that you should design modules that never change. When requirements change, you extend the behavior of such modules by adding new code, not by changing old code that already works.

The Open-Closed Principle, Robert C. Martin

Again, like all the SOLID principles, this is straight-forward in its definition but more complex in its implementation. Let’s look at each of the areas this relates to so we can get a more through understanding of the issues.

Abstraction

First off we have to abstract any specific implementations away from the base class into the child classes.

Example 1: The “Shape” Abstraction done wrong (based loosely on the source reference)

public enum ShapeType { Circle, Square }
public abstract class Shape
{
	ShapeType _type;
	public ShapeType Type { get { return _type; } }

	protected Shape(ShapeType Type)
	{
		this._type = Type;
	}
}

public class Circle : Shape
{
	double _radius;
	public double Radius { get { return _radius; } }

	Point _center;
	public Point Center { get { return _center; } }

	public Circle(double Radius, Point Center)
		: base(ShapeType.Circle)
	{
		this._radius = Radius;
		this._center = Center;
	}
}

public class Square : Shape
{
	double _side;
	public double Side { get { return _side; } }

	Point _topLeft;
	public Point TopLeft { get { return _topLeft; } }

	public Square(double Side, Point TopLeft)
		: base(ShapeType.Square)
	{
		this._side = Side;
		this._topLeft = TopLeft;
	}
}

public class Canvas
{
	public void DrawAllShapes(List<Shape> Shapes)
	{
		foreach (Shape shape in Shapes)
		{
			switch (shape.Type)
			{
				case ShapeType.Circle:
					DrawCircle((Circle)shape);
					break;
				case ShapeType.Square:
					DrawSquare((Square)shape);
					break;
			}
		}
	}

	void DrawCircle(Circle Circle) { throw new NotImplementedException(); }
	void DrawSquare(Square Square) { throw new NotImplementedException(); }
}

The most obvious problem here is that you can’t extend Shape without making major modifications to Canvas to support other shape types. Not only would you have to make changes to “DrawAllShapes” but you would have to add new methods for each new type to draw it. Remember too that this is a fairly contrived case. In real code you would have switch cases all over the place trying to deal with each ShapeType that would end up being a serious case of spaghetti code. You shouldn’t have to make changes to dependent modules anytime you want to extend an existing class. In the same vein is the ShapeType, where anytime we add a new shape we would have to add a new ShapeType.

Example 2: OOD solution to Square/Circle problem. (based loosely on the source reference)

public abstract class Shape
{
	abstract public void Draw();
}

public class Circle : Shape
{
	double _radius;
	public double Radius { get { return _radius; } }

	Point _center;
	public Point Center { get { return _center; } }

	public Circle(double Radius, Point Center)
	{
		this._radius = Radius;
		this._center = Center;
	}

	public override void Draw() {/* Draw ourself */}
}

public class Square : Shape
{
	double _side;
	public double Side { get { return _side; } }

	Point _topLeft;
	public Point TopLeft { get { return _topLeft; } }

	public Square(double Side, Point TopLeft)
	{
		this._side = Side;
		this._topLeft = TopLeft;
	}

	public override void Draw() {/* Draw ourself */}
}

public class Canvas
{
	public void DrawAllShapes(List<Shape> Shapes)
	{
		foreach (Shape shape in Shapes)
		{
			shape.Draw();
		}
	}
}

We’ve removed the need to define a shape type and abstracted “Draw” away from canvas and into each class. This means that each class can be extended with its own draw method without having to modify DrawAllShapes. There a problem here however. What if you may need to define a property/member variable or method in such a way that is should be done as an abstract in the base class? Well, you suck it up, make changes to your unit tests to cover this new case and modify any and all classes affected by this change assuming you can. When you do this you risk braking functionality any clients are dependent on. Case in point, my sample model from my series on the TPL and MVVM. When I initially wrote my TPL sampler there was no need for a “ImageRequired” property as all samples I had envisioned used an image and there was no reason not to reset and reload the image before each sample was run. The problem arose that reloading the image each time was a resource heavy and IO blocking operation, especially on large files and I quickly came to samples where they didn’t need an image but instead did other operations to show how their functionality worked.

The first solution some developers might think to do is simply modify the top-level call where it resets and reloads the image with a case/switch such that if it is one type of class, reload the image, if it is another then don’t reload it. But this is very bad and actually violates another part of this principle I’ll discuss later. The solution was to add an abstract ImageRequired property to the base model and then go back to each of my models and add in this property so that it could be used without having to modify the code where the image for the samples was reset and reloaded.

Referring back to the source material it goes into further ways of achieving closure that I’ll leave as an exercise to the reader. The primary sample it contains is, “What if we have to draw shapes in a paticular order on our canvas?” and goes into how to achieve this. The end result is that, probably the best way to do this is order the classes based on a “OrderTable” where the classes are ordered by how they should be drawn. This way type is still abstracted away from the classes themselves and dependent modules don’t have to change but can rely on the “OrderTable”. OrderTable is still dependent on the class types but not on the classes themselves and so can operated independently. I know it sounds like a lot of handwaving to try and make this all work for functionality like this but having a single place that defines stuff like this that is independent can make maintenance a lot easier.

Heuristics and Conventions

There are a lot of “Good Dev Practices” that are done just to facilitate the OCP and Uncle Bob mentions a few of the more important ones. I’ll touch briefly on these but I would recommend reading the source material for a more in-depth coverage of each one.

Make all Member Variables Private

This where the argument that member variables should only be accessed via getters/setters comes from. This way the class that implements the member variable has control over it. We don’t want the potential that the meaning of what a variable is changes by the classes that use it external to the class that implements it. The source material provides an example to this end. In .NET this is made rather simple for us by using properties, though using “PropertyName{get;set;}” just exposes the member variable as if it were public. I argued in a post from over 6 years ago that we should strive for immutability. If you look at the classes in the Shape example above you will see that the classes are immutable with private member variables and public getters. It’s important to note as I’ve mentioned that just implementing properties with the default {get;set;} violates this principle as it just exposes the private member variable with no thought to how it is used or accessed.

No Global Variables — Ever

I have seen pretty consistently where the meaning of global variables changes over time. This means that early code uses a global variable in one manner but future code uses it in another manner. Not only that but we run into the problem where one class or module could change the variable in a way that is unexpected from other modules using it. It violates the OCP because by exposing the global we allow for it to be changed over time. This means that code that relies on the global in one manner will have to be changed if the meaning of the global changes over time. Uncle Bob does mention a case for Global Variables. Where the definition of those variables can’t change, like in cout and cin (or in C#, Console.Write() or Console.Read()). These are static global instances where they perform a functionality but we can’t change them.

The most consistent violation of this is the use of the Singleton pattern where there is but a single global reference to a resource (like Console). This is done intentionally with the idea that control over the resource is needed and more important (or more convenient). In the case of a singleton, the object should not be changed and therefore adheres to the OCP. I’ll cover the good and bad of Singleton when I do coverage on patterns.

RTTI is Dangerous

RTTI is run time type identification (RTTI). In C# it is the use of the “is” keyword or using “as” and checking for null.

Example 3: Using RTTI in Example 1 for our Canvas.DrawAllShapes()

public class Canvas
{
	public void DrawAllShapes(List<Shape> Shapes)
	{
		foreach (Shape shape in Shapes)
		{
			if (shape is Circle)
			{
				DrawCircle((Circle)shape);
			}
			else if (shape is Square)
			{
				DrawSquare((Square)shape);
			}
		}
	}
	
	void DrawCircle(Circle Circle) { throw new NotImplementedException(); }
	void DrawSquare(Square Square) { throw new NotImplementedException(); }
}

This is great because we eliminate the dependency on ShapeType. We’ve cleaned up the code a bit and made maintenance easier. So why is this bad? Because you will still have to modify Canvas for any new shapes that need to be added. Example 1 was an example of what not to do for a reason and by using RTTI all we’ve done is make it look like it should be okay to implement things poorly. So when is it okay to use RTTI?

Exmaple 4: Using RTTI in Example 2 where it does not violate the OCP (based loosely on the source reference)

public class Canvas
{
	public void DrawAllShapes(List<Shape> Shapes)
	{
		foreach (Shape shape in Shapes)
		{
			shape.Draw();
		}
	}

	public void DrawSquares(List<Shape> Shapes)
	{
		foreach (Shape shape in Shapes)
		{
			if(shape is Square)
				shape.Draw();
		}
	}
}

Here we’ve added “DrawSquares”. All we’re doing is drawing squares. It makes sense to verify that we have a square before we try and draw it. This doesn’t violate the OCP because even if the new shapes are added we don’t have to go back and modify “DrawSquares”. It will always do just what it says it will do.

That’s it on the OCP. As with all the principles there are times where you have to balance adhering to the rules with other considerations but when in doubt it’s generally better to follow the rules. Up next is the Liskov Substitution Principle.

Thanks,
Brian

Previous post in this series:
SOLID – (SRP) The Single Responsibility Principle

Some of my source links in the previous two posts may not have worked in FireFox (but worked fine in IE and Chrome). These links have been fixed. It was an issue with how google was handling the redirects to the source files.

In Uncle Bob’s source material for SRP it mentions one specific example of where it doesn’t make sense to break out the responsibilities.

Here is a modified class based on the source material:

public class ConnectionManager<K, V>
{
	void Connect(K EndPoint);
	void Terminate();
	void Send(V Data);
	V Receive();
}

In the source material the object is of a modem with dial and hangup methods but this is a simplified version of that. You can see here that there are really two responsibilities in this interface. The first is that there is connection management. Connect and Terminate are one set of responsibilities that relate to handling the connection. Send and Receive don’t really have anything to do with handling the connection, they have to do with the data flow to/from the end point.

Here are the interfaces separated (based on source material):

public class ConnectionManager<K, V>
{
	public IConnection<K> Connection { get; set; }
	public IDataChannel<V> DataChannel { get; set; }
}
public interface IConnection<T>
{
	void Connect(T EndPoint);
	void Terminate();
}
public interface IDataChannel<T>
{
	void Send(T Data);
	T Receive();
}

Should these clearly different responsibilities need to be separated? You can read Uncle Bob’s answer in the source material. My answer? It’s a tough situation to define without a context. If this were just dealing with connecting via a single type of modem directly where we can think small I wouldn’t decouple. You end up with what Uncle Bob calls, “Needless Complexity.” A modem connects, sends and receives data back and forth and at some point disconnects. The code should be able to handle this cleanly.

Where the problem arises is that most of the time we aren’t dealing with such simple situations. If we do have a complex situation but we’ve implemented our code as in the first sample we end up with what Uncle Bob calls a, “smell of Rigidity.” For instance, what if we have a scenario where we need to send and receive multiple types? Do we create multiple connections even though the end points are the same? Are we going to have to support multiple DataChannels? In the first example trying to expand on the functionality becomes a pain and I often feel constrained in these cases.

All this comes down to a point I’ve emphasized here before. As stated in “Code Complete” one of the most important things you can do that will reap the most benefit is a thorough requirements analysis. If you fully understand the use cases you can develop for them. Then once you fully understand your requirements you can know if it makes sense to separate responsibilities.

Up next is the Open-Closed Principle (OCP).

Thanks,
Brian

Previous post in this series:
SOLID – Getting Started and Source Material

I’m sure most of us have taken an Object Oriented Programming (OOP) class in college (or will soon). What is important to understand is that an OOP class will teach you the fundamentals of understanding the principles of developing from an object-centric perspective. Uncle Bob’s SOLID principles takes those concepts from OOP and develops them further into object oriented design (OOD), how each object should be developed so that long term maintenance of software is easier, cleaner and ultimately cheaper.

The SRP is one of the simplest of the principles, and one of the hardest to get right. Conjoining responsibilities is something that we do naturally. Finding and separating those responsibilities from one another is much of what software design is really about. Indeed, the rest of the principles we will discuss come back to this issue in one way or another.

Agile Principles, Patterns, and Practices in C#, Robert C. Martin

SRP is pretty easy right? An object should only have one responsibility. As an example lets look at a Vehicle, the age old example of OOP classes everywhere. What is the responsibility of a vehicle? It is used to transport people and/or things.

What do we need to know about the vehicle so it can accomplish it’s responsibility?

1. How many people or things it can hold?
2. Is it capable of transporting those things?
• Does it have at least one entry to allow for putting things in and out?
• Does it have the ability to move (i.e. tires or sleds)?
• Does it have a working means of propulsion?

What don’t we care about?

1. How big is the engine?
2. How does the engine work?
3. What does it look like?
4. How many tires does it have (if any)?
5. What kind of tires are they?

As Uncle Bob mentions, we naturally want to conjoin responsibilities. We want to put everything in the vehicle and call it a day. But a vehicle shouldn’t define what an engine is, how it works and what it does. It may define how it uses that engine but the engine it’s self shouldn’t be defined within the vehicle.

That’s an important part to the SRP. It doesn’t say an object can’t contain other objects that facilitate it’s responsibility. An engine isn’t a vehicle and a vehicle isn’t an engine. A vehicle shouldn’t define what an engine is within it’s self. There is nothing wrong, however, with it have a Engine property.

To try and make this a bit clearer:

The Single Responsibility Principle (SRP) says that a class should have one, and only one, reason to change. To say this a different way, the methods of a class should change for the same reasons, they should not be affected by different forces that change at different rates

SRP in Ruby, Robert C. Martin

This (slightly modified) code sample from Microsoft on rectangles gives a good example of this:

<Window x:Class="Wpf_Test.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        Title="MainWindow">
    <Canvas>
        <Path Stroke="Black" StrokeThickness="1" Fill="LemonChiffon">
            <Path.Data>
                <RectangleGeometry>
                    <RectangleGeometry.Rect>
                        <Rect X="10" Y="100" Width="150" Height="100" />
                    </RectangleGeometry.Rect>
                </RectangleGeometry>
            </Path.Data>
        </Path>
    </Canvas>
</Window>

All this does is draw a rectangle with the class RectangleGeometry. What is important here is that there is a property, Rect, that defines the the properties of where a rectangle is and how big it is. The struct, Rect, has a bunch of operations that have nothing to do with drawing a rectangle on the canvas, methods like IntersectsWith(Rect) and Union(Rect). Whereas RectangleGeometry has Freeze() and GetRenderBounds() that have to do with drawing the rectangle on the screen but have nothing to do with directly working with rectangles. Thus a RectangleGeometry has a Rect property to take care of these things.

So know we have the instructions on the paint can. But when do not listen to them? That’s up in the next post.

Thanks,
Brian

This next series will follow the principles of SOLID, “five basic principles of object-oriented programming and design.”

The concepts as collated into SOLID was organized by a gentleman named Robert C. Martin who has what may be one of my favorite quotes in software engineering:

Following the rules on the paint can won’t teach you how to paint.

This is an important point. Principles will not turn a bad programmer into a good programmer. Principles have to be applied with judgement. If they are applied by rote it is just as bad as if they are not applied at all.

Having said that, if you want to paint well, I suggest you learn the rules on the paint can. You may not agree with them all. You may not always apply the ones you do agree with. But you’d better know them. Knowledge of the principles and patterns gives you the justification to decide when and where to apply them. If you don’t know them, your decisions are much more arbitrary.

Getting a SOLID start, Robert C. Martin

Why is this one of my favorites? I’ll get to that in the future (when I get to patterns).

I will be referring to Robert C. Martin through out these posts as Uncle Bob. This isn’t intended to imply any familiarization between myself and Uncle Bob, nor any familial relationship. In fact, quite the opposite, I’ve never met Uncle Bob. But Uncle Bob self identifies as “Uncle Bob” using it for his old blog, as a twitter handle (@UncleBobMartin), and for his email. I figure he wants us to call him Uncle Bob, so why not?

Here are the references I’ll be utilizing on the posts covering each of the SOLID principles (ripped almost literally from Uncle Bob’s own post on object oriented design):

Needless to say, the vast majority of the content for this series will be based on Uncle Bob’s writings. I will try to make it clear the content that belongs to Uncle Bob and content that belongs to me and cite appropriately as I’ve done for posts in the past when I reference someone else’s work. This series is a case of “dwarfs standing on the shoulders of giants” taking Uncle Bob’s work and extending it to encapsulate my own experience with each of these topics.

So coming next week will be the first SOLID principle, “The Single Responsibility Principle”.

Thanks,
Brian