Sunday, August 30, 2009

Single Responsibility Principle – discovering violations

Single Responsibility Principle states:

A class should have only one reason to change. Responsibilities are reasons to change.

Violations of this principle leave you face to face with fragile design and all the maintenance nightmares it implies. Unfortunately there is no hundred percent way to prevent it from happening – it is just the nature of design.

Common sense often helps to spot multiple responsibilities mixed within a single class. However some cases are tricky and require careful attention. This is where design assessment comes in. An object can be looked at from two perspectives:

  • Data
    • What it knows?
    • What connections between objects it maintains?
  • Behavior
    • What it decides?
    • What services it performs?

Uncontrolled growth in any of the groups can lead to god class (data or behavior forms respectively).

What it knows?

Keeping related data and behavior in one place is essential to building consistent abstractions. Failing to do so may lead to:

  • unnecessary details disclosure (by exposing all the data necessary to implement behavior)
  • duplicated or inconsistent behavior (due to poor discoverability)
  • uncontrolled growth (as it is harder to see the big picture due to scattered behavior)

Following Information Expert pattern promotes the idea:

Assign a responsibility to the class that has the information needed to fulfill it.

Consider an example below where publicly visible person name could be incorrectly used to distinguish people (as names are not unique). Thus it makes sense to let Person type to define what equality means as it has all the necessary information (social security number). This will prevent introduction of several inconsistent equality behaviors across the code base.

class Person
{
    private string name;
    // Keep social security number in private
    private string ssn;
    
    // Names are not unique
    public string Name { get { return name; } }

    public override bool Equals(object obj)
    {
        if (obj == null || obj.GetType() != typeof(Person))
            return false;
        // Names cannot be used to identify people unlike 
        // social security number
        return ssn == ((Person) obj).ssn;
    }
}

Keeping related data and behavior in one place is as important as not letting not related sets of data/behavior to be put into the same class. Most of the methods defined on a class should be using most of the data members most of the time.

Many of you started to work still being a student. Employer needs your abilities to work and most likely he isn’t interested in that you still studying.

// Smart student - works and studies
class Student
{
    private string name;
    private int knowledge;
    private Func<Course, bool> preferences;
    private int experience;

    public string Name
    {
        get { return name;}
    }

    public void Study()
    {
        knowledge++;
    }

    public void Enlist(IEnumerable<Course> courses)
    {
        // Select appropriate courses and enlist
        foreach (var course in courses.Where(preferences))
            course.Enlist(name);
    }

    public void Work()
    {
        experience++;
    }

    public void Sign(Contract contract)
    {
        // sign job contract with your name
        contract.Agree(name);
    }
}

This class clearly has more than one responsibility and the fact that study related methods and work related methods both operate on a subset of data shows that. However separation can solve the problem.

class Student
{
    private Person person;
    private int knowledge;
    private Func<Course, bool> preferences;

    public Student(Person newStudent)
    {
        person = newStudent;
    }

    public void Study()
    {
        knowledge++;
    }

    public void Enlist(IEnumerable<Course> courses)
    {
        // Select appropriate courses and enlist
        foreach(var course in courses.Where(preferences))
            course.Enlist(person.Name);
    }
}

class Employee
{
    private Person person;
    private int experience;

    public Employee(Person newEmployee)
    {
        person = newEmployee;
    }

    public void Work()
    {
        experience++;
    }

    public void Sign(Contract contract)
    {
        // sign job contract with person's name
        contract.Agree(person.Name);
    }
}

What connections between objects it maintains?

What is connection between objects anyway? For example, a document can be seen a set of glyphs (text, graphics, structural elements like columns and rows). The connection between them is the fact they belong to the same document. So basically the document maintains connection between glyphs. So what must attract our attention in this area to spot Single Responsibility Principle violations?

If the object maintains connections between objects of different abstraction levels it is highly likely that it does someone’s job.

Consider an object model where a man wants to build a house. Man class maintains connection between tasks (got from an architect) and workers to build a house.  And this where different abstraction levels (from domain model perspective) meet – a man hired foreman who is supposed to manage construction but it so happens that a man wants to control everything and treats hired foreman as a simple worker giving him tasks to do in not necessarily correct order. Basically a man got foreman’s responsibility (or in other words foreman abstraction isn't consistent).

abstract class Task
{
    public bool Completed { get; private set; }

    public virtual void Do()
    {
        DoCore();
        Completed = true;
    }

    protected abstract void DoCore();
}

class Worker
{
    public virtual bool CanDo(Task task)
    {
        // Can do everything worker =)
        return true;
    }

    public virtual void Accept(Task task)
    {
        task.Do();
    }
}

class Foreman : Worker
{
    IEnumerable<Worker> workers;
    Func<Task, bool> next;

    public override bool CanDo(Task task)
    {
        // Can do things only on time or otherwise the house 
        // won't stand long
        return next(task);
    }  

    public override void Accept(Task task)
    {
        // Foremen looks for a worker who can do the task
        workers.Where(w => w.CanDo(task)).First().Accept(task);
    }
}

// A man who tries to manage his new house construction
class Man
{
    IEnumerable<Task> construction;
    Foreman foremen;

    public void BuildHouse()
    {
        IEnumerable<Task> tasks;
        // As far as we don't know what to do next we'll ask foreman
        while ((tasks = construction.Where(t => foremen.CanDo(t) && !t.Completed)).Any())
        {
            foreach (var task in tasks)
            {
                foremen.Accept(task);
            }
        }
    }
}

A man must let foreman to manage tasks or become a foreman himself (we’ll take first approach or otherwise the house won't stand long).

class Foreman
{
    IEnumerable<Worker> workers;
    Func<Task, bool> next;
    
    public void Manage(IEnumerable<Task> construction)
    {
        IEnumerable<Task> tasks;
        // Foreman selects tasks that are not completed and must be done next
        while((tasks = construction.Where(t => !t.Completed && next(t))).Any())
        {
            foreach (var task in tasks)
            {
                workers.Where(w => w.CanDo(task)).First().Accept(task);
            }
        }
    }
}

class Man
{
    IEnumerable<Task> construction;
    Foreman foremen;

    public void BuildHouse()
    {
        // Foreman takes the whole construction to manage
        foremen.Manage(construction);
        // Just check that everything is done
        if (construction.Where(t => !t.Completed).Any())
        {
            throw new InvalidOperationException("Something isn't done!");
        }
    }
}

Now abstractions are leveled - man allows construction to be managed by foreman (foreman abstraction is now consistent).

What it decides?

It is quite uncomfortable when someone asks you for your opinion and then makes his own mind and tells you what and how to do. Why don’t tell what to do in the first place and let me decide how to do it. Isn’t that looking like you are doing my job? This is what Tell Don’t Ask Principle is about:

As the caller, you should not make decisions based on the called object’s state and then change the object’s state.

Here is couple of teenager definitions – which do you think makes more sense?

class Teenager
{
    private int age;
    private List<string> clothes;

    // Is it age that drives your clothes preferences?
    public int Age { get { return age; } }

    // Do you really wan't your parents to dress you 
    // just based on your age?
    public void TakeNew(string clothing)
    {
        clothes.Add(clothing);
    }
}

// ... or

class Teenager
{
    private List<string> clothes;
    // Preference is something personal
    private Func<string, bool> preference;

    // And you want to able to select clothes based on your 
    // preferences?
    public bool SelectNew(IEnumerable<string> shop)
    {
        var clothing = shop.Where(preference).FirstOrDefault();
        if (clothing != null)
        {
            // Got something you like
            clothes.Add(clothing);
        }
        // Tell your parents whether you need to go to other shop =)
        return clothing != null;
    }
}

By violating Tell Don’t Ask principle caller gains responsibility on make decision instead of the called one. Busted!

What services it performs?

Service provider can do the job by itself or ask collaborators for help. Keeping dependencies on collaborators explicit facilitate controlled class growth. Classes should not contain more objects than a developer can fit in his or her short-term memory (5-7). Otherwise it is possible to introduce responsibilities into class which are either duplicated or not related. Whenever number of collaborators growth more than 5-7 you should consider whether all collaborators still help you to form single abstraction or a new one was introduced.

On the other hand it makes sense to look at consumers and in particular on how they consume supplied services. If consumers use different subsets of provided services it may be a sign that class captures more than one responsibility.

Recall the student example. Original class has two consumers: employer and university (for example). Both consumers were interested in a subset of provided methods (working and studying related respectively). And as we discovered the class captured two abstractions: employee and student.

Summary:

  • DO pay careful attention to responsibilities assignment
  • CONSIDER decomposing class into areas (what it knows, maintains, does and decides) and performing assessment in each area

Thursday, August 13, 2009

Inject or locate dependencies?

Inversion of Control pattern allows to decouple components (consumers) from their dependencies and takes care of dependencies location and lifetime management through delegation of these responsibilities to external (with respect to dependent type) component. This pattern actively used in composite application development.

Inversion of Control comes in two flavors (Unity provides both capabilities):

Service locator holds references to services and knows how to locate them. It is further used by dependent component to obtain necessary services. In other words consumers play active role.

interface IService
{
    void Do();
}

class ActiveConsumer
{
    private readonly IUnityContainer locator;

    // Reference to service locator comes from outside
    public ActiveConsumer(IUnityContainer serviceLocator)
    {
        locator = serviceLocator;
    }

    public void Do()
    {
        // In order to fulfill its task active consumer relies 
        // on service implementation that is obtained on demand 
        // from service locator
        var service = locator.Resolve<IService>();
        service.Do();
    }
}

Dependency injection makes dependent components passive (little or no work is done to get its dependencies). The only responsibility consumers still care about is to express their dependencies somehow (the way dependencies are expressed depends on pattern implementation, but for this example we will use single constructor automatic injection supported by Unity).

class PassiveConsumer
{
    private readonly IService service;

    // This constructor is used to inject service dependency
    public PassiveConsumer(IService svc)
    {
        service = svc;
    }

    public void Do()
    {
        // We got this dependency from outside and done nothing 
        // to let it happen - so just use it
        service.Do();
    }
}

...

// At this point container resolves consumer's dependency 
// and injects it during construction 
var passiveConsumer = container.Resolve<PassiveConsumer>();
passiveConsumer.Do();

So what is the difference?

First, is dependency from service locator appropriate? If the component in question is supposed to be reused by others you may end up with putting unnecessary constraints (for example you are using some open source service locator but developers that could reuse your component are not allowed to use any open source stuff due to customer’s demand and thus won’t be able to reuse the component).

Second, dependencies visibility. Service locator makes consumer’s “real” dependencies hidden and dependency from service locator itself visible. When dependencies are explicit it is much easier to understand dependent class. Explicit dependencies allows you to assess and control the growth of the component. For example, if your component accepts 10 services in its constructor it may be a sign that it does, or knows or decides too much and it is time to split it. Consider the same thing when using service locator. In order for you to spot number of dependencies you need to look for all unique usage occurrences of service locator. It is not that hard with modern IDE but still it is not that easy as looking at component’s contract.

On the other hand, it makes sense to consider the audience of the component. If it will be reused by others and dependencies are hidden it may require deep knowledge of component’s inner workings in order to use it.

Third, consumer’s relation with dependency. Dependency injection promotes constant relations (from lifetime perspective). Consumer obtains its dependency at construction time and lives with it. On the other hand service locator compasses to temporary relations – get service instance when it is time, call its methods, discard it. Why discard? Because if the component has a constant relation why not use dependency injection otherwise which gives you explicit dependencies?

But anyway, what particular case forces locator usage? When consumer has longer lifetime than its dependency. For example, you are writing smart client application. You organized presentation layer using Model-View-Presenter pattern. Presenter calls remote service in response to user interaction. View controlled by a presenter can be opened for a long time. If presenter gets remote service proxy dependency only once it may happen that proxy will go into faulted state (for example, due to network connectivity problems) and any subsequent calls to it will result in exception. So it is better to dispose proxy every time a particular task accomplished and create new one when new task is on the way or cache it and in response to proxy going faulted create a new one (which is of course harder as long as you need to handle all cases where it used and maintain cache). Thus it seems that service locator is more appropriate in this case.

However we can make short lived dependencies explicit. Let’s assume that IService implementation instance must be disposed every time it is used.

interface IService : IDisposable
{
    void Do();
}

// This is still active consumer as it uses service locator to get service instance
class Consumer
{
    private readonly IUnityContainer locator;

    public ActiveConsumer(IUnityContainer serviceLocator)
    {
        locator = serviceLocator;
    }

    public void Do()
    {
        using (var service = locator.Resolve<IService>())
        {
            service.Do();
        }
    }
}

Service locator has wide surface (it terms of services it can provide) and this makes consumer’s contract opaque. What we need to do is narrow the surface but still provide ability to create service instances (as long as we need to dispose them every time). Abstract factory will do the thing. Factory provides clear connection with service it creates. On the other hand we need to make consumer’s dependency from factory explicit. We will use dependency injection.

interface IServiceFactory
{
    IService CreateService();
}

// Consumer is no longer active as it gets its dependencies from outside
class Consumer
{
    private readonly IServiceFactory factory;

    // The dependency is now explicit
    public PassiveConsumer(IServiceFactory serviceFactory)
    {
        factory = serviceFactory;
    }

    public void Do()
    {
        // We still can create service instances on demand
        using (var service = factory.CreateService())
        {
            service.Do();
        }
    }
}

How about this? That is not all. Factory clearly and explicitly states the relation between dependent component and dependency (service that is created by factory) – it is a temporary relation (as long as it provides ability to create new instances).

Summary:

  • DO make dependencies explicit
  • DO use dependencies to assess and control growth of the dependent component
  • CONSIDER component audience when choosing between dependency injection and service locator
  • CONSIDER using abstract factory pattern and dependency injection to make short lived dependencies (in comparison with dependent component lifetime) explicit

Wednesday, August 5, 2009

Type parameter leveling

At some point developers have to decide whether to use generic class or generic method (or in other words whether to put type parameter on type level or method level).

Type parameter at type level (generic type type parameter) allows to enforce constraints for multiple members that use this type parameter. For example, IEnumerable<T> fixes what kind of enumerators it can produce to IEnumerator<T>.

public interface IEnumerable<T> : IEnumerable
{
    IEnumerator<T> GetEnumerator();
}

Generic type may be used when it:

  • implements generic interface
  • has internal state that depends on type parameter
  • type parameter is implicitly used as an internal state

You can also look at the alternatives from granularity point of view. Type parameter of a generic type has wider influence than type parameter of generic method. If type parameter makes sense only for a single member it doesn’t makes sense to put type parameter at a higher level than member that uses it. Otherwise you will force consumers of members to depend on type parameter and its constrains they may not use.

static class Util<T>
    where T: IComparable<T>
{
    public static T Min(T left, T right)
    {
        return left.CompareTo(right) < 0 ? left : right;
    }
    
    public static void Swap(ref T left, ref T right)
    {
        var tmp = left;
        left = right;
        right = tmp;
    }
}

...

int i1 = 10, i2 = 15;
var min = Util<int>.Min(i1, i2);

object o1 = i1; object o2 = i2;
// Not allowed and results in compilation error
Util<object>.Swap(ref o1, ref o2);

In the case above type parameter introduces constrain which makes sense only for Min method and thus limits capabilities of Swap method (Swap can operate on any argument and Min can only operate on comparable arguments). On the other hand generic type argument must be specified explicitly and it doesn’t use advantages of type inference which simplifies usability of the API.

static class Util
{
    public static T Min<T>(T left, T right)
        where T: IComparable<T>
    {
        return left.CompareTo(right) < 0 ? left : right;
    }

    public static void Swap<T>(ref T left, ref T right)
    {
        var tmp = left;
        left = right;
        right = tmp;
    }
}

...

int i1 = 10, i2 = 15;
var min = Util.Min(i1, i2);

object o1 = i1; object o2 = i2;
// Now it is perfectly legal
Util.Swap(ref o1, ref o2);

Changing type parameter level with respect to members that depend on it within inheritance hierarchy may lead to Design by Contract and Liskov Substitution Principle violation.

I came across an API which do the trick recently. In the abstracted example below ISomething states that its DoSomethingWith method can do something with enumerable of any T.

interface ISomething
{
    void DoSomethingWith<T>(IEnumerable<T> target);
}

But the implementation provides specialization on a fixed type parameter. It changed type parameter level from method to type level – DoSomethingWith method implementation depends on generic type type parameter whereas interface states method’s dependency from method type parameter.

class FixedSomething<U> : ISomething
{
    private Func<U, bool> predicate;
    private IEnumerable<U> slice = Enumerable.Empty<U>();

    public void DoSomethingWith<T>(IEnumerable<T> target)
    {
        if (typeof (U) != typeof (T))
            // Cannot do something on argument of type T. U type is expected.
            throw new ArgumentException();

        slice = target.Cast<U>().Where(predicate).Concat(slice);
    }

    //... other members elided for clarity 
}

It violates ISomething contract as it strengthens input parameter type constrain and introduces new exception that may be thrown. On the other hand it performs runtime type check which violates LSP.

Summary:

  • DO use generic type when it implements generic interface or has internal state that depends on type parameter.
  • CONSIDER type parameter granularity.
  • DO NOT change type parameter level within inheritance hierarchy.