Design Review and Delegation Vs. Inheritance

This week I want to discuss another bullet from my code review checklist. 

Consider delegation over inheritance.  I have had some pretty heated debates on this topic in the past and I will even take it a step further and say the rule should state that we should favor delegation over inheritance.  Let’s consider the following classic (and intuitive) person/student/teacher example.  A class diagram might look something like this

Person is our base class; Teacher and student derive from person.  We can say that a student Is A person and that a Teacher Is A person.

The up-side of this design is it is simple, intuitive, and it will perform well.  The down side of this approach is that there is a tight coupling between the derived classes and the base class, we are breaking encapsulation by making the functionality of the derived class dependent on the functionality in the base class, and we may even break encapsulation on our Person class by using protected or public access modifiers where we normally wouldn’t do so. 

The problem with breaking encapsulation is that making a change to one class (our base class) can cause unintended changes in other consuming classes (our derived classes).  I can’t begin to tell you how many times I have seen a seemingly small change to a base class break huge pieces of an application.

Now let’s consider using delegation rather than inheritance.  We will change our class diagram to look like the following:

We can say a teacher Has A person and similarly student Has A person.

The down-side of this design is that we have to write more code to create and manage the person class and thus the code will not perform as well (though in most cases I suspect the performance hit is negligible).  We also cannot use polymorphism - we cannot treat a student as a person and we cannot treat a teacher as a person.  The up-side of this design is that we can decrease coupling by defining a person interface and using the interface as the return type for our Person property rather than the concrete Person class, and our design is more robust.  When we use delegation we can have a single instance of a person act as both a student and a teacher.  Try that in C# using the inheritance design!

 

Technorati Tags: Design Review,inheritance,delegation c# code review

Design Review and the DRY Principle

DRY in the DRY principle is an acronym for Don’t Repeat Yourself.  While we are going to focus on applying the principle to code, it can and should be applied to more than just code.  The can be applied to database schemas, tests, documentation, test plans, design documents, build scripts and more.  Any time we start duplicating our work we are signing up for parallel maintenance and chances are we will eventually have the items fall out of synch.

We can use several techniques to avoid duplicate code.  Some obvious things we can do include using classes and methods to organize common code.  We can write an abstract or base class to implement common behaviors for multiple derived classes. 

We can use properties to perform a validation in one place rather than performing the validation anywhere we want to use a member variable:

Code Snippet

1.         [System.Diagnostics.DebuggerBrowsable(System.Diagnostics.DebuggerBrowsableState.Never)]
2.         int aComplexInteger;
3.         public int AComplexInteger
4.         {
5.             get { return aComplexInteger; }
6.             set
7.             {
8.                 if (value == 0)
9.                     throw new ArgumentOutOfRangeException("AComplexInteger");
10.                 if (value != aComplexInteger)
11.                 {
12.                     aComplexInteger = value;
13.                     //Maybe raise a value changed event
14.                 }
15.             }
16.         \

One of my favorite techniques is constructor chaining.  If we have multiple constructors that perform similar logic , we should use constructor chaining to avoid duplicating code:

Code Snippet

1. using System;
2. using System.Collections.Generic;
3. using System.Linq;
4. using System.Text;
6. namespace CicMaster
7. {
8.     class BetterConstructorLogic
9.     {
10.         #region WhyItIsBetter
11.         //No duplicate code, this is called constructor chaining
12.         //step through this
13.         #endregion
14.         string someString = string.Empty;
15.         int someInteger = 0;
16.         List<int> myIntegers = new List<int>();
18.         public BetterConstructorLogic():this("A Default Value")
19.         {
20.             //someString = "A Default Value";
21.             System.Diagnostics.Debug.WriteLine("In Default Constructor");
22.         }
24.         public BetterConstructorLogic(string aString):this(aString,123)
25.         {
26.             //someInteger = 123;
27.             System.Diagnostics.Debug.WriteLine("In one param constructor");
28.         }
30.         public BetterConstructorLogic(string aString, int anInteger)
31.         {
32.             System.Diagnostics.Debug.WriteLine("In two param constructor");
34.             someString = aString;
35.             someInteger = anInteger;
36.             myIntegers.Add(someInteger);
37.             myIntegers.Add(someInteger ^ 3);
38.             myIntegers.Add((int)(2 * 3.14 * someInteger));
39.         }
40.     }
41. }

The final technique I would like to mention is use a factory to create all but the simplest objects.  The following (admittedly nonsensical) code needs to execute maybe a half dozen lines of code to construct an Order object. 

Code Snippet

1. using System;
2. using System.Collections.Generic;
3. using System.Linq;
4. using System.Text;
5. using System.Threading;
7. namespace BusinessLayer
8. {
9.     class ObjectContext
10.     {
11.         public ObjectContext(String username)
12.         {
13.             //Look up the user permissions
14.         }
15.         public bool IsInTranaction { get; set; }
16.         public bool BeginTransaction()
17.         {
18.             //do some transaction logic
19.             return true;
20.         }
21.     }
22.     class Order
23.     {
24.         public Order(ObjectContext theContext)
25.         {
27.         }
29.     }
30.     class Consumer
31.     {
32.         public Consumer()
33.         {
34.             ObjectContext ctx = new ObjectContext(Thread.CurrentPrincipal.Identity.Name);
35.             if (!ctx.IsInTranaction)
36.             {
37.                 if (ctx.BeginTransaction())
38.                 {
39.                     //...
40.                 }
41.             }
42.             Order order = new Order(ctx);
44.         }
45.     }
47. }

Duplicating these few lines of code in a couple places is not that difficult.  Now say the application is enhanced and grows for a few years and suddenly we see this code duplicated dozens or hundreds of times.  At some point it is likely that we want to change the construction logic, finding and changing all the code we use to create the order is difficult, time consuming, and a QA burden. 

A better approach would be to encapsulate the logic required to build a new order.  Here is an implementation using a simple factory.  It is much easier to find, change, and test this code:

Code Snippet

1.     static class OrderFactory
2.     {
3.         public static Order GetOrder()
4.         {
5.             ObjectContext ctx = new ObjectContext(Thread.CurrentPrincipal.Identity.Name);
6.             if (!ctx.IsInTranaction)
7.             {
8.                 if (ctx.BeginTransaction())
9.                 {
10.                     //...
11.                 }
12.             }
13.             return new Order(ctx);
14.         }
15.     }
16.     class DryConsumer
17.     {
18.         public DryConsumer()
19.         {
20.             Order order = OrderFactory.GetOrder();
21.         }
22.     \

If we recognize that we are duplicating even a few lines of code over and over we need to take a serious look at the code and figure out a way to encapsulate the logic.

 

My design review checklist

Technorati Tags: DRY,Design Review,Agile principles,Don't Repeat Yourself