Design Review and the Open/Closed Principle

In my last blog I talked about the Single Responsibility Principle. SRP it is an easy concept to understand but breaking down a system into the “correct” objects is difficult to do well. It is imprecise and there is a lot of room for opinions. You seldom know the correct breakdown until you are finished. In this blog I want to take a look at the OCP or Open/Closed Principle. The Open/Closed principle states that Software entities (classes, modules, functions etc.) should be open for extension but closed for change. (“Agile Principles, Patterns, And Practices in C#” Robert C Martin and Micah Martin). In other words this means that we would like to be able to change the behavior of existing entities without changing the code or binary assembly. This concept is not as easy to understand. Did I just say to change something without changing it?

Let’s use a configurable rule engine as an example. Assume the simple rule engine has 2 types of entities, a Condition and an Action. The Action is simple, execute some logic and return true on success or false on failure. The condition is also simple, it checks some binary condition and returns true or false. The rules engine takes a list of conditions and actions and executes them. If an Action returns false, the rules stop executing and a rollback is performed. If a condition returns true, the next action is executed. If the condition is false, the next action is skipped and the following action is executed. Pretty simple so let’s look at some sample code:

Define our Conditions, Actions, and Rule Type with enums

Code Snippet

1.     public enum Actions
2.     {
3.         CreateOrder,
4.         CreateBackorder,
5.         CloseOrder,
6.         ShipOrder,
7.         StoreOrder,
8.         ReduceInventory
9.     }
10.     public enum Conditions
11.     {
12.         IsComplete,
13.         IsInStock,
14.         CanShip
15.     }
16.     public enum RuleType
17.     {
18.         Condition,
19.         Action
20.     \

Here is a very crude implementation of a rule engine

Code Snippet

1.     public class RuleEngine
2.     {
3.         
4.         public void ExecuteRules(int rulesId)
5.         {
6.             //Gather all the conditions and actions
7.             //assume we have a Ruleset table that looks like this:
8.             //ID - Int
9.             //Index - Int
10.             //RuleType - Int (corresponds to RuleTypeEnum)
11.             //Rule - Int (corresponds to Conditions or Actions enum)
13.             //Note, transaction logic, creating the parameter
14.             //Error handling, and safe DataReader.Read() logic
15.             //has been omitted for brevity
16.             using (SqlConnection connection = new SqlConnection("ConnectString"))
17.             {
18.                 using ( SqlCommand cmd = new SqlCommand
19.                     ("SELECT RuleType, Rule FROM Ruleset where ID = ? ORDER BY Index"
20.                     , connection))
21.                 {
22.                     SqlDataReader rules = cmd.ExecuteReader();
23.                     bool executing = true;
24.                     while (executing)
25.                     {
26.                         
27.                         rules.Read();
28.                         if ((int)rules[0] == (int)RuleType.Action)
29.                         {
30.                             executing = ExecuteAction((int)rules[1]);
31.                         }
32.                         else
33.                         {
34.                             if(ExecuteCondition((int)(rules[1])))
35.                             {
36.                                 executing = true;
37.                             }
38.                             else
39.                             {
40.                                 rules.Read();
41.                                 executing = true;
42.                             }
43.                         }
44.                         //Implement some exit strategy here
45.                     }
46.                 }
47.             }
48.         }
49.         public bool ExecuteCondition(int theCondition)
50.         {
51.             switch (theCondition)
52.             {
53.                 case (int)Conditions.IsComplete:
54.                     //Is the order complete?
55.                     return true;            //or false
56.                 case (int)Conditions.CanShip:
57.                     //Can we ship the order?
58.                     return true;            //or false
59.                 case (int)Conditions.IsInStock:
60.                     //Is this item in stock?
61.                     return true;            //or false
62.                 default:
63.                     throw new Exception("Unsupported Condition");
64.             }
65.         }
66.         
67.         public bool ExecuteAction(int theAction)
68.         {
69.             switch (theAction)
70.             {
71.                 case (int)Actions.CreateOrder:
72.                     //Execute order create logic
73.                     return true;            //or false
74.                 case (int)Actions.CreateBackorder:
75.                     //Execute backorder logic
76.                     return true;            //or false
77.                 case (int)Actions.ShipOrder:
78.                     //Execute shipping logic
79.                     return true;            //or false
80.                 case (int)Actions.StoreOrder:
81.                     //send to warehouse
82.                     return true;            //or false
83.                 case (int)Actions.CloseOrder:
84.                     //Execute order close logic
85.                     return true;            //or false
86.                 case (int)Actions.ReduceInventory:
87.                     //Remove item from inventory
88.                     return true;            //or false
89.                 default:
90.                     throw new Exception("Unsupported Action");
91.             }
93.         }
94.     \

Never mind the problems with error handling, transactions, and the lack of support for nested conditions, the point of the blog is OCP, not creating a rule engine. What we have will work but does it satisfy the OCP? No, there are a couple problems with this type of implementation that will make it difficult to maintain. The first problem is that the logic for the Rule Engine, Conditions, and Actions are all in one class and therefore one assembly. Any system that wants to use any of this logic will be tied to all of this logic. The second problem is that any time you want the rule engine to do something new, you have to modify this assembly which is a violation of the Open/Closed Principle.

Let’s take a look at a more robust design.

We will still use an enum to distinguish between Actions and Conditions:

Code Snippet

1.     public enum RuleType
2.     {
3.         Condition,
4.         Action
5.     \

Now let’s declare an interface

Code Snippet

1.     public interface ISupportRules
2.     {
3.         public bool Execute();
4.         public RuleType TypeOfRule();
5.     \

We will put both the enum and the Interface in an assembly called Rule.Types.

Now lets add a few classes

Code Snippet

1.     public class CreateOrderAction:ISupportRules
2.     {
3.         #region ISupportRules Members
5.         public bool Execute()
6.         {
7.             //Order Create Logic Here
8.         }
10.         public RuleType TypeOfRule()
11.         {
12.             return RuleType.Action;
13.         }
15.         #endregion
16.     \

We will put this in an assembly called Rule.Actions.CreateOrder.

Code Snippet

1.     public class CanShipCondition:ISupportRules
2.     {
3.         #region ISupportRules Members
5.         public bool Execute()
6.         {
7.             //Execute Can Ship Logic here
8.         }
10.         public RuleType TypeOfRule()
11.         {
12.             return RuleType.Condition;
13.         }
15.         #endregion
16.     \

We will put this class in Rules.Conditions.CanShip.

In fact we will create a separate assembly for each condition and action we defined in the enums.

Here is our new rules engine which goes in the Rule.Engine assembly:

Code Snippet

1.         public void ExecuteRules(List<ISupportRules> rules)
2.         {
3.             bool executing = true;
4.             int ruleIndex = 0;
5.             while (executing)
6.             {
7.                 if (rules[ruleIndex].TypeOfRule() == RuleType.Action)
8.                 {
9.                     executing = rules[ruleIndex].Execute();
10.                     ruleIndex++;
11.                 }
12.                 else
13.                 {
14.                     if (rules[ruleIndex].Execute())
15.                     {
16.                         ruleIndex++;
17.                         executing = true;
18.                     }
19.                     else
20.                     {
21.                         ruleIndex += 2;
22.                         executing = true;
23.                     }
24.                 }
25.                 //Implement some exit strategy here
26.             }
28.         }
29.     \

Notice that the ExecuteRules method takes a generic list of type ISupportRules as a parameter but has no reference to any of the conditions or actions. Also notice that the condition and action classes have no reference to each other or the rules engine. This is key to both code reuse and extensibility. the refactored rule engine, condition, and action classes are completely independent of each other. All they share is a reference to Rule.Type. Some other system may use any of these assemblies independent of each other with the only caveat being they will need to reference the Rule.Type assembly. The other thing we gained with this approach is we can now Extend the rule engine (make it execute new conditions and actions) by simply adding a new Condition or Action that implements ISupportRules and passing it into the ExecuteRules method as a part of the generic list. We can do all of this without recompiling the RefactoredRulesEngine which is the goal of the OCP. By the way, this design approach is called the Strategy Pattern.

If you haven’t noticed yet I’m leaving out one major piece of the puzzle. How does the generic list of rules get generated? I’m going to wave my hands here a little bit and save the details for another blog. We would use a creational pattern (one of the factory patterns). If we assume we are consuming the table outlined in our first solution this factory would accept a Ruleset ID and magically return the generic List<IsupportRules> of rules. The implementation of the factory pattern could be written in such a way that each time you add a Condition or an Action the factory would need to be recompiled or we could use a provider pattern and use a configuration file to allow us to create these new Conditions and Actions without a recompile.

To summarize things a bit: conceptually we have this RulesEngine that is relatively complex (much more complex than I have written) and we want to write it, test it, and leave it alone. At the same time though we have this need to enhance the system by adding more rules. By using using the strategy pattern we now have this stable rule execution engine that can execute any condition or action that implements the ISupprotRules interface. Because we inject a list of conditions and rules into the ExecuteRules method we can do all of this without recompiling the refactored rules engine. Another approach we might have taken to satisfy the OCP is the Template Method pattern. In the template method pattern we would make use of an abstract class to define the skeleton of an algorithm, then allow the concrete classes to implement subclass specific operations.

My design review checklist

One of my favorite design pattern web sites: http://www.dofactory.com/Patterns/Patterns.aspx