Ditching the Switch/Case

The other day I was exploring some code that I was preparing to edit in order to provide new functionality. It is a web service that takes values from the calling application, populates a PDF with the values, and publishes the PDF to deliver to the calling application. These are legal documents, so there are a large number of fields on each of the documents to populate. Each of those fields is mapped to a space on the PDF template in an XML metadata file. The trick is using the metadata to map the input to the PDF. My predecessor took the obvious route, and this is how I found the data mapping was being done:

 private void SetFieldValue(PdfStamper writer, Field field, PdfInput input)
{
    if (!writer.AcroFields.Fields.ContainsKey(field.FieldName))
    {
        return;
    }
    switch (field.PropertyName)
    {
        case "Is12MonthLoanChecked":
            if (input.Is12MonthLoanChecked)
            {
                writer.AcroFields.SetField(field.FieldName, GetCheckedState(writer, field.FieldName));
            }
            else
            {
                writer.AcroFields.SetField(field.FieldName, GetUncheckedState(writer, field.FieldName));
            }
            break;
        case "Is24MonthLoanChecked":
            if (input.Is24MonthLoanChecked)
            {
                writer.AcroFields.SetField(field.FieldName, GetCheckedState(writer, field.FieldName));
            }
            else
            {
                writer.AcroFields.SetField(field.FieldName, GetUncheckedState(writer, field.FieldName));
            }
            break;
        case "Is36MonthLoanChecked":
            if (input.Is36MonthLoanChecked)
            {
                writer.AcroFields.SetField(field.FieldName, GetCheckedState(writer, field.FieldName));
            }
            else
            {
                writer.AcroFields.SetField(field.FieldName, GetUncheckedState(writer, field.FieldName));
            }
            break;

     ......

This switch case continues for another 66 cases, and this is only the mapping for one of the documents. Although this works perfectly well, two things bothered me about this solution:

  1. Due to the number of cases and the use of strings for the cases, this implementation creates a potential maintenance nightmare of misspelled magic strings that are not unit testable.
  2. It just made me feel dirty.

As a big proponent of The Boy Scout Rule, I had to make sure this was not in the code base when I was done adding the newest entry fields. I was uninspired on solutions and so I searched the internet and found an article by Chris Brandsma at ElegantCode.com called Refactoring A Switch Statement. Brandsma’s solution is elegant, but I still found problems with the management of the delegates that do the “work.” Brandsma uses a dictionary that maps the cases to delegates that fulfill each case. This was better but still required the management of a large number of strings arranged in a way which is barely testable and open for spelling mistakes. i.e.:

 public Dictionary<string, Func<string, string>> CreateDictionary()
{
    var dictionary = new Dictionary<string, Func<string, string>>
                         {
                             {"Chris", _valueProcessor.Chris},
                             {"David", _valueProcessor.David},
                             {"Jason", _valueProcessor.Jason},
                             {"Scott", _valueProcessor.Scott},
                             {"Tony", _valueProcessor.Tony}
                         };
    return dictionary;
}
 

Remembering the introduction of the CallerMemberName Attribute in the .NET from my WPF days, and how it reduced the problems with the use of magic strings with the FirePropertyChanged() method, I decided the best way to make this refactor testable was to create a custom attribute so that I could use reflection to build my delegate map at run time. The custom attribute is very simple:

[AttributeUsage(AttributeTargets.Method)]
public class FieldProcessorMethodName : Attribute
{
    private readonly string _name;

    public FieldProcessorMethodName(string name)
    {
        _name = name;
    }
 
    public string Name { get { return _name; } }
}

I added this attribute to each of my delegates, assigning the delegate the name of the specified element in the XML metadata file:

[FieldProcessorMethodName("Is12MonthLoanChecked")]
internal void Set12MonthLoanCheckBoxState(PdfStamper writer, Field field, PdfInput input)
{
    SetCheckboxState(writer, field, input.Is12MonthLoanChecked);
}

This allowed the creation of my delegate mapping through reflection by doing the following:

public Dictionary<string, FieldProcessorDelegate> CreateDelegateDictionary()
{
    var delegateDictionary = new Dictionary<string, FieldProcessorDelegate>();

    var classMethods = GetType().GetMethods(BindingFlags.Instance | BindingFlags.NonPublic);
 
    foreach (var method in classMethods)
    {
        var attributes = method.GetCustomAttributes(true);
        foreach (var attribute in attributes)
        {
            var fieldProcessorNameAttribute = attribute as FieldProcessorMethodName;

            if (fieldProcessorNameAttribute != null)
            {
                var fieldAlias = fieldProcessorNameAttribute.Name;
                var fieldDelegate = (FieldProcessorDelegate) Delegate.CreateDelegate(typeof (FieldProcessorDelegate), this, method);
 
                delegateDictionary.Add(fieldAlias,fieldDelegate);
             }
        }
    }
 
    return delegateDictionary;
}

This method reduced the number of places that needed to be updated every time a field is added to the PDF from 4 places to 2 places, and it allowed me to test to make sure the spelling of the element names was correct in the code by checking each of the keys in the dictionary against the list of element names generated from the XML file.

Now if I can generalize the PdfInput from each of the PDFs, I can apply this approach to the case statements of all the other PDF generation logic.

Advertisements