Convention over Configuration

Convention over is defined in Wikipedia as follow:

Convention over Configuration (aka Coding by convention) is a software design paradigm which seeks to decrease the number of decisions that developers need to make, gaining simplicity, but not necessarily losing flexibility.

Our current project, that is no longer a skinny cow, has a state machine of message statuses, and a factory that can produce the state object from a key. Today I ran into a bug, when added two new message states, but forgot to update the factory, that happened to reside in a completely different assembly from the one that contains state classes. After a quick trace the bug was found. But how painful it is:

  1. As a developer I had to keep in mind to link between new message state classes and message state factory. This is getting easily out of control when you either new to the project or have not worked on it for a while. New developers have to be familiar with the limitation or find it out the difficult way.
  2. Each time a message state is added or removed to the state machine, factory has to be updated.
  3. Message state factory specification (tests) has to be updated as the production code, and again that’s a manual process that can lead to forgetfulness.

The conclusion was “we have to have A Pit of Success”. Convention over Configuration (or simply CoC) is what we need. So I spiked this quickly (no specs, sorry for that) and this is what is looks like.

Defining a “State”

public abstract class BaseState
{
  public abstract string Name { get; }

public virtual BaseState MoveToNextState()
{
throw new NotImplementedException(string.Format("State '{0}' is not implemented.", Name));
}
}

Base abstract BaseState class defines the default behavior and attribute for state name. A few state (three for simplicity) with only ability to move forward would look like the following code snippets.

image

public classInitialState : BaseState
{


  public const stringStateName = "Initial";



  public override stringName


  {


    get{ returnStateName; }


  }



  public overrideBaseState MoveToNextState()


  {


    return newIntermediateState();


  }


}


public class IntermediateState : BaseState
{
  public const string StateName = "Intermediate";

  public override string Name
  {
    get { return StateName; }
  }

  public override BaseState MoveToNextState()
  {
    return new FinalState();
  }
}

public class FinalState : BaseState
{
  public const string StateName = "Final";

  public override string Name
  {
    get { return StateName; }
  }
}

Fairly simple and straightforward. State factory, normally, would be a collection of all states, with a getter behavior that would allow from a key to get the state object. For simplicity, I am omitting the case when requested key has no equivalent state object.

public class StateFactory
{
  private static readonly Dictionary<string, BaseState> dictionary = new Dictionary<string, BaseState>
                                                                       {
                                                                          {InitialState.StateName, new InitialState()},
                                                                          {IntermediateState.StateName, new IntermediateState()},
                                                                          {FinalState.StateName, new FinalState()},
                                                                       };

  public BaseState GetStateFromName(string stateName)
  {
    return dictionary[stateName];
  }
}

This is the spot where the disconnection has happened. Each time a state is added or removed, dictionary has to be updates as well. So end this up, we can leverage CoC to reflectively discover statuses, and prevent future manual updates and forgetfulness (thanks David for a nice way of putting it in words).

Reflective Discoverer

The contract for reflective discoverer would accept 3 Funcs

  1. Function to return an array of assemblies to scan, determined at the time of invocation.
  2. Function to generate a key value, based on the item type.
  3. Function to verify is a type is matching the required criteria (could be predicate).
public interface IReflectiveDiscoverer
{
  Dictionary<key_type, item_type> Discover<key_type, item_type>(Func<Assembly[]> assembliesToScan,
                                                                Func<item_type, key_type> generateKeyFrom,
                                                                Func<Type, bool> verifyTypeCanBeUsed);
}

Implementation of the contract:

public class ReflectiveDiscoverer : IReflectiveDiscoverer
{
  public Dictionary<key_type, item_type> Discover<key_type, item_type>(Func<Assembly[]> assemblies_to_scan,
                                                                       Func<item_type, key_type> generate_key_from,
                                                                       Func<Type, bool> verify_type_can_be_used)
  {
    var dictionary = new Dictionary<key_type, item_type>();

    foreach (var assembly in assemblies_to_scan())
    {
      var all_types_in_assembly = assembly.GetTypes().Where(verify_type_can_be_used);

      foreach (var candidate_type in all_types_in_assembly)
      {
        var instance = (item_type)Activator.CreateInstance(candidate_type);
        dictionary.Add(generate_key_from(instance), instance);
      }
    }

    return dictionary;
  }
}

Now it’s time to update StateFactory to use ReflectiveDiscoverer.

public class StateFactory
{
  public BaseState GetStateFromName(string stateName)
  {
    var states = new ReflectiveDiscoverer().Discover<string, BaseState>(() => AppDomain.CurrentDomain.GetAssemblies(),
                                                                        state => state.Name,
                                                                        type => type.IsSubclassOf(typeof (BaseState)))
    return states[stateName];
  }
}

 

A few implementation details missing in this version:

  1. Factory is used frequently, and probably should keep states cached.
  2. We skipped specifications, and therefore have un-testable code.

Fixing those issues is not difficult. For testability we need to inject IReflectiveDiscoverer. For performance issue – keep the dictionary cached.

Final Version

private static Dictionary<string, BaseState> states;

public StateFactory() : this(new ReflectiveDiscoverer()) {}

public StateFactory(IReflectiveDiscoverer reflectiveDiscoverer)
{
  if (states != null) return;

  states = reflectiveDiscoverer.Discover<string, BaseState>(() => AppDomain.CurrentDomain.GetAssemblies(),
                                                            state => state.Name,
                                                            type => type.IsSubclassOf(typeof (BaseState)));
}

public BaseState GetStateFromName(string stateName)
{
  return states[stateName];
}

Parameterless constructor is replacing container functionality. “states” dictionary is a static member field that caches states with a guard clause in constructor.

Now, the code is cleaner and automated. No need to add states when those are added or removed. This will ensure that developers don’t make mistake of forgetting to update factory.

Finally, if we developers embrace the concept “People don’t fail, processes do”, our code will be better.

No Comments