Refactoring challenge Part 2 – Preparation

Other posts in this series:

• Refactoring challenge – cry for help
• Part 1 – Examination

In the last part of this series, I took a closer look at the code smells found by commenters, which included:

• No tests
• Feature envy
• Conditional complexity
• Long method

All of these code smells can be found in the two main refactoring books out there, Fowler’s Refactoring and Kerievsky’s Refactoring to Patterns.  A while back, I created a handy smells-to-refactorings quick reference guide for those sticky situations where I don’t have the books handy.  But before I address any of the code smells, I need to prepare for the refactorings.  The main refactorings I’m looking at are ones to move towards the Strategy and Chain of Responsibility patterns, but these only really become apparent when the code at hand is rather uniform.

To fix this problem, I first need to flatten out all of those if statements in the original code.  The code after flattening all the if statements out now looks like this:

public object Map(ResolutionContext context, IMappingEngineRunner mapper)
{
    object mappedObject = null;
    var profileConfiguration = mapper.ConfigurationProvider.GetProfileConfiguration(context.TypeMap.Profile);

    if (context.TypeMap.CustomMapper != null)
    {
        context.TypeMap.BeforeMap(context.SourceValue, mappedObject);
        mappedObject = context.TypeMap.CustomMapper(context);
    }
    else if ((context.SourceValue == null && profileConfiguration.MapNullSourceValuesAsNull))
    {
        mappedObject = null;
    }
    else if (context.DestinationValue == null && context.InstanceCache.ContainsKey(context))
    {
        mappedObject = context.InstanceCache[context];
    }
    else if (context.DestinationValue == null)
    {
        mappedObject = mapper.CreateObject(context.DestinationType);
        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);
        }
    }
    else
    {
        mappedObject = context.DestinationValue;

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


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

By flattening my conditional statements, I can now much more easily apply other refactorings.  But in the process, I wound up creating more duplication, so we need to amend our code smell list.  With our updated list, here’s the planned refactorings:

• Feature envy – Extract/Move method
• Conditional complexity – Replace conditional logic with strategy/chain of responsibility
• Long method – Same as above
• Duplicated code – Form template method

It turns out at one point I had to do the exact same refactorings on another large piece in AutoMapper, one that chose different mapping algorithms to use (type maps, direct assignment, arrays, dictionaries, lists, etc.).  In the next post, I’ll fix the feature envy code smell.