My struggles in understanding and learning about Object Oriented design, and the tools and knowledge I've taken from them.

Monday, January 3, 2011

The Open-Closed Principle

The Open-Closed Principle states, quite concisely, that classes should be "Open for extension, but closed for modification." In other words, the attributes and behaviors of a class (ie the building blocks of a class) should be thoughtfully placed where they belong in the inheritance chain. I don't think I could really overstate how many times I've violated this principle. In fact, in looking back at code I've written in the past, I see all kinds of examples where I violate it. But this is definitely one of the principles that has strongly impacted how I write code.

The example Uncle Bob used to describe this principle was a Shape example. The implementation of the Open-Closed is via inheritance, and eventually, polymorphism.

Martin starts with an antipattern to describe Open-Closed. Below is an example in C# that is similar to Martin's:

We start with some base citizens in our code base:

 
public class Shape
{
double Area;
///...Other shape code
}

public class Square : Shape
{
///...Square code
}

public class Circle : Shape
{
///....Circle code
}

public class SomeImplementation
{
public void DrawCircle()
{
///...Draw a circle
}

public void DrawSquare()
{
///...Draw a square
}
///...Some code in Some Implementation

public void DrawAllShapes(List shapeList)
{
for(int i = 0; i < shapeList.Count; i++)
{
Shape currentShape = shapeList[i];
if(currentShape is Square)
DrawSquare();
else if(currentShape is Circle)
DrawCircle();
}
}
}


The above code violates the Open-Closed Principle because it could not accomodate new shapes. So, adding new shapes would entail making a change to DrawAllShapes() every time a new shape got added to the program's vernacular. In the original article, Shape, Square, and Circle were Structs, not Classes, which made the antipattern much more ugly-looking. I didn't use a struct in my example, because overusing Structs was never a real problem in my learning how to program.

The better solution, which does not violate the Open-Closed Principle is shown below:

 
public abstract class Shape
{
double Area;

public abstract void Draw();

///...Other shape code
}

public class Square : Shape
{
public override void Draw()
{
///...Do Square drawing
}
}

public class Circle : Shape
{
public override void Draw()
{
///...Do Circle drawing
}

}

public class SomeImplementation
{
///...Some code in Some Implementation

public void DrawAllShapes(List shapeList)
{
foreach(Shape currentShape in shapeList)
currentShape.Draw();
}
}



In the above code, the SomeImplementation class uses polymorphism to draw any shape in the passed shape list. So, as more shapes are added to the application, the DrawAllShapes() method does not need to change (the Closed portion of the principle), and the new shape can simply be a new class, inherited from Shape, or one of its children (the Open portion of the principle).

As a personal aside, I used to violate this principle all the time, because I would create "Collection classes" that would inherit from a List object, and every time a need for a new type of the same collection occurred, I would add a new constructor to represent that type of collection; the better solution would have been to inherit from the base collection. So, the below code is what I used to do:

 
public class SomeClass
{
}

public class SomeClassCollection : List
{
Category _category;
Person _person;

public class SomeClassCollection()
{
_category = null;
_person = null;
populate();
}

public class SomeClassCollection(Category category)
{
_category = category;
_person = null;
populate();
}

public class SomeClassCollection(Person person)
{
_person = person;
_category = null;
populate();
}

private void populate()
{
if(_category == null && _person == null)
{
///Do type 1 collection population
}
else if(_category == null && _person != null)
{
///Do type 2 collection population
}
else if(_category != null && _person == null)
{
///Do type 3 collection population
}
}
}


The better way to build the above code to avoid violating the Open-Closed Principle would have been to extend SomeClassCollection:

 
public class SomeClassCollection : List
{

protected virtual void populate()
{

}
}

public CategoryCollection : SomeClassCollection
{
public CategoryCollection(Category category)
{

}
override void populate()
{
///Populate based on category
}
}

public CategoryCollection : SomeClassCollection
{
public CategoryCollection(Person person)
{

}
override void populate()
{
///Populate based on person
}
}

No comments:

Followers

Search This Blog

Powered by Blogger.