Refactoring challenge #2 – functionally illiterate

In the last refactoring challenge, I had a problem with some nasty conditional complexity.  To be honest, the challenge was a subtle way to crowdsource new features in AutoMapper, but hey, it worked.  One of the hard parts of being a sole contributor to an OSS project is that I lose the benefits of teammates and pair programming, to be able to bounce design ideas around and brainstorm solutions.  So instead, I’ll just post my problems here.

The next issue I’m trying to address is the ability to configure the matching algorithm AutoMapper uses to determine that member Foo matches up to another member Foo on two separate types.  Right off the bat, we could simply match by name.  But, AutoMapper has one killer piece of functionality, the ability to flatten source member names to destination member names, so that “src.Foo.Bar.Bazz.Bam” can map to “dest.FooBarBazzBam”, or “dest.FooBarBazz.Bam”, or “dest.FooBar.BazzBam”, or “dest.Foo.Bar.Bazz.Bam”.  But in taking Ayende’s advice to JFHCI, I wound up hardcoding a lot of the algorithm to match as well.  It’s all encapsulated in one class, but it’s not easy to change.

To get to where I want to go, such as being able to supply postfixes to accept (src.CustomerId matching to dest.Customer), I need to shore up the searching algorithm to make it more understandable.  All mapping configuration is contained in a TypeMap class, created by this method:

public TypeMap CreateTypeMap(Type sourceType, Type destinationType)
{
    var sourceTypeInfo = GetTypeInfo(sourceType);
    var destTypeInfo = GetTypeInfo(destinationType);

    var typeMap = new TypeMap(sourceTypeInfo, destTypeInfo);

    foreach (IMemberAccessor destProperty in destTypeInfo.GetPublicReadAccessors())
    {
        var resolvers = new LinkedList<IValueResolver>();

        if (MapDestinationPropertyToSource(resolvers, sourceTypeInfo, destProperty.Name))
        {
            typeMap.AddPropertyMap(destProperty, resolvers);
        }
    }
    return typeMap;
}

This isn’t the ugly part, it’s the MapDestinationPropertyToSource that’s the beast.  But as you can see above, all I really do is loop through the destination properties, trying to find a match on the source type.  But again like I had with the mapping execution, I have a bit of nastiness in the matching:

private bool MapDestinationPropertyToSource(LinkedList<IValueResolver> resolvers, TypeInfo sourceType, string nameToSearch)
{
    var sourceProperties = sourceType.GetPublicReadAccessors();
    var sourceNoArgMethods = sourceType.GetPublicNoArgMethods();

    IValueResolver resolver = FindTypeMember(sourceProperties, sourceNoArgMethods, nameToSearch);

    bool foundMatch = resolver != null;

    if (!foundMatch)
    {
        string[] matches = Regex.Matches(nameToSearch, @"p{Lu}[p{Ll}0-9]*")
            .Cast<Match>()
            .Select(m => m.Value)
            .ToArray();

        for (int i = 0; i < matches.Length - 1; i++)
        {
            NameSnippet snippet = CreateNameSnippet(matches, i);

            IMemberAccessor valueResolver = FindTypeMember(sourceProperties, sourceNoArgMethods, snippet.First);

            if (valueResolver == null)
            {
                continue;
            }

            resolvers.AddLast(valueResolver);

            foundMatch = MapDestinationPropertyToSource(resolvers, GetTypeInfo(valueResolver.MemberType), snippet.Second);

            if (foundMatch)
            {
                break;
            }

            resolvers.RemoveLast();
        }
    }
    else
    {
        resolvers.AddLast(resolver);
    }

    return foundMatch;
}

Again, problems with conditional complexity, and even more, cyclomatic complexity with the loop continuation/breaking.  The actual matching is quite easy to do:

private static IMemberAccessor FindTypeMember(IEnumerable<IMemberAccessor> modelProperties, IEnumerable<MethodInfo> getMethods, string nameToSearch)
{
    IMemberAccessor pi = modelProperties.FirstOrDefault(prop => NameMatches(prop.Name, nameToSearch));
    if (pi != null)
        return pi;

    string getName = "Get" + nameToSearch;
    MethodInfo mi = getMethods.FirstOrDefault(m => (NameMatches(m.Name, getName)) || NameMatches(m.Name, nameToSearch));
    if (mi != null)
        return new MethodAccessor(mi);

    return null;
}

private static bool NameMatches(string memberName, string nameToMatch)
{
    return String.Compare(memberName, nameToMatch, StringComparison.OrdinalIgnoreCase) == 0;
}

private static NameSnippet CreateNameSnippet(IEnumerable<string> matches, int i)
{
    return new NameSnippet
               {
                   First = String.Concat(matches.Take(i + 1).ToArray()),
                   Second = String.Concat(matches.Skip(i + 1).ToArray())
               };
}

private class NameSnippet
{
    public string First { get; set; }
    public string Second { get; set; }
}

I’ve hardcoded the method prefix option (i.e., GetValue() maps to a property called Value), and that would be easy to switch out.  But since I’m in the middle of needing to change all of the matching algorithm, I figured I’d go ahead and take care of the MapDestinationPropertyToSource.  It’s quite complex, with the looping, linked list, and recursion.  But algorithms are definitely not my strong suit, more modeling and patterns.

Given the recursive nature of the matching algorithm, I’m assuming that a better solution exists, but’s it’s probably functional in nature.  Unfortunately, I’m functionally illiterate, but I’m sure some more enterprising folks have a better solution.  So here’s the challenge – fix the “MapDestinationPropertyToSource” method, so it actually makes some sense when I come back to it later.