Refactoring Challenge Part 3 – Pattern Implementation


In the previous part to the refactoring challenge, I needed to structure the original implementation to a point where I could start applying other refactorings.  Whenever I start to see a bunch of “if” statements or a big switch statement, this is a pretty strong smell that I need to reduce conditional complexity by moving towards some specific patterns.  The big one I’m looking for is a combination of Strategy and Specification, mashed together in some sort of pipeline.  The idea is to allow each strategy be responsible for both the decision to do work, and the work itself.  You wind up with an interface something like:

public interface IStrategySpec
{
    bool IsMatch(SomeContext context);
    void DoWork(SomeContext context);
}

If this looks familiar, I blogged a while back on how we do model binding in our MVC applications.  It’s the same pattern:

public interface IFilteredModelBinder
{
    bool IsMatch(Type modelType);
    Object BindModel(ControllerContext controllerContext, 
        ModelBindingContext bindingContext);
}

And our convention-based input builders follow again the same pattern:

public interface IInputBuilder
{
    bool IsMatch(IInputSpec inputSpec);
    string BuiltHtml(IInputSpec inputSpec);
}

By separating the implementation of each strategy from how it’s used, I can allow each design to grow independently.  For example, in the input builder example, we can apply all sorts of design patterns to IInputBuilders, such as Template Method, Decorator and so on, and not change the underlying infrastructure that actually handles all of our IInputBuilders.

Back to our example, in my original method, I had two responsibilities in each conditional block:

  • Deciding if the ResolutionContext was a match for a specific scenario
  • If a match, execute some mapping strategy

So first, I’ll need something to encapsulate the original strategy, my IStrategySpec.  That’s a terrible name, so I picked something a little more meaningful:

public interface ITypeMapObjectMapper
{
    object Map(ResolutionContext context, IMappingEngineRunner mapper);
    bool IsMatch(ResolutionContext context, IMappingEngineRunner mapper);
}

With my interface defined, I can create the individual mapping implementations.

Creating the individual mappers

This part was especially easy, it’s just a matter of combining Extract Method and Move Method to move each conditional block and its corresponding mapping logic out to an individual class.  For example, here is one that concerns using a custom mapping function:

public class CustomMapperStrategy : ITypeMapObjectMapper
{
    public object Map(ResolutionContext context, IMappingEngineRunner mapper)
    {
        return context.TypeMap.CustomMapper(context);
    }

    public bool IsMatch(ResolutionContext context, IMappingEngineRunner mapper)
    {
        return context.TypeMap.CustomMapper != null;
    }
}

It’s quite simple, but one class encapsulates all logic and behavior concerning mapping using a custom mapping function.  The other mappers were pulled out as well.  One interesting scenario I ran into was where two mapping algorithms were very similar.  I could then apply Template Method so that derived types only need to supply the pieces that changed:

public abstract class PropertyMapMappingStrategy : ITypeMapObjectMapper
{
    public object Map(ResolutionContext context, IMappingEngineRunner mapper)
    {
        var mappedObject = GetMappedObject(context, mapper);
        if (context.SourceValue != null)
            context.InstanceCache.Add(context, mappedObject);

        context.TypeMap.BeforeMap(context.SourceValue, mappedObject);
        foreach (PropertyMap propertyMap in context.TypeMap.GetPropertyMaps())
        {
            MapPropertyValue(context, mapper, mappedObject, propertyMap);
        }

        return mappedObject;
    }

    public abstract bool IsMatch(ResolutionContext context, IMappingEngineRunner mapper);

    protected abstract object GetMappedObject(ResolutionContext context, IMappingEngineRunner mapper);

This is a great example where because I picked a powerful seam of integration to begin with, I’m left with many more options for designing the individual mapping behaviors.

Once all my mappers were created, I now needed to refactor the original class to use these individual strategy classes instead.

Implementing the pipeline

Instead of a bunch of conditional logic, all I really need now is a collection of mappers, and some logic to loop through them.  The final mapper became quite small:

public class TypeMapMapper : IObjectMapper
{
    private readonly IEnumerable<ITypeMapObjectMapper> _mappers;

    public TypeMapMapper(IEnumerable<ITypeMapObjectMapper> mappers)
    {
        _mappers = mappers;
    }

    public object Map(ResolutionContext context, IMappingEngineRunner mapper)
    {
        var mapperToUse = _mappers.First(objectMapper => objectMapper.IsMatch(context, mapper));
        object mappedObject = mapperToUse.Map(context, mapper);

        context.TypeMap.AfterMap(context.SourceValue, mappedObject);
        return mappedObject;
    }

    public bool IsMatch(ResolutionContext context)
    {
        return context.TypeMap != null;
    }
}

That’s it!  I loop through all my dependent mapper, picking the first one that matches the context passed in.  Next, I use that mapper to map from the source object to some destination object.  AutoMapper also supports custom callbacks for before/after mapping events, so I call any potential callback before finally returning the mapped object.

With the responsibilities clearly separated between selecting and applying the mapping, and the actual conditions and mapping algorithms, I’ve allowed my design to grow in ways that won’t add to the overall complexity.  In the original method, I would cringe if I needed to add more behavior to this specific mapping algorithm, it was just too complex.  After the refactoring, it’s a lot more understandable as I can group and name each mapping algorithm separately.

So this area is a lot cleaner – but no rest for the weary, feature requests are causing me to go back to other places that need some tidying up.  On a side note, I often get the question “When should I refactor?”  I’m not a fan of refactoring code just because I feel like it – there has to be a need.  The need is change.  If code needs to change, that’s when it’s time to refactor.  I needed to add a feature to the above class, so it was time to clean it up.

When is Poor Man’s Dependency Injection appropriate?