Refactoring Switch Statements in C#

I’m currently in the middle of Robert C. Martins Clean Code: A handbook of Agile Software Craftsmanship where he points out that he’s not a fan of switch statements. Now I agree with the reasoning behind not using them but sometimes when I’m short on time and just need the code to work, I’ll use them. Ofcourse there are times when a simple switch statment does the job so I’ll avoid refactoring them, just for the sake of refactoring. The problem I find is that after a while, those switch statements start growing to a point where you can’t see the logic on one screen without scrolling. Time to refactor them at that point.

Background

The test in question navigates to a page that contains various contact information for a given Case and simply replaces (edits) the details with new values. The new values are randomly generated from a Data Generator class that I’ve written. The test also randomly selects which contact information to edit (you can tell I like randomness 🙂 ) so each run will edit different information. To capture the contact information, I’ve got a ‘Contacts’ class that contains properties for each of the fields on the page. Below is a sample of the properties I can fill/edit.

public class Contacts 
{ 
    public string Telephone { get; set;} 
    public string Mobile { get; set;} 
    public string Email{ get; set;} 
    … 
}

The wire frame below gives you an idea of what the webpage looks like. It’s simply a grid with contact details that can be edited for various people. There are other functions that take place but we’ll concentrate on editing the fields visible below.

Contacts wireframe

The code that interacts with the page is below:

var contacts = ContactDetailsPage.GetAllContacts();
foreach(var contact in contacts)
{
    var newContact = Contacts.CreateRandomContact();
    var rnd = DataGenerator.RandomInt(1, 3);
    ContactDetailsPage.EditDetails(contact.GetContactType(), 
         contact.Name, 
         rnd,      
         newContact);
}

The code is fairly self-explanatory, I retrieve a collection of contacts from the webpage & loop through them. Each iteration creates a new contact with randomly generated details. This contact will be used to edit the information that is displayed on the web page.  The next line generates a number between 1 & 3 that is then passed to the EditDetails class. This class decides which field to edit based on the values passed into it.

public void EditDetails(string contactType, string contactName,
int fieldToEdit, Contacts newContact)
{
    var row = FindContactRowByTypeAndName(contactType, contactName);
    switch (fieldToEdit)
    {
        case 1:
        {
            var telephone = row.FindElement(By.XPath("td[3]/div/Input"));
            telephone.Clear();
            telephone.SendKeys(newContact.Telephone);
            break;
        }
        case 2:
        {
            var mobile = row.FindElement(By.XPath("td[4]/div/Input"));
            mobile.Clear();
            mobile.SendKeys(newContact.Mobile);
            break;
        }
        …
        default:
            Assert.Fail("Unknown contact type to edit : " + contactType);
            break;
        }
    }
}

Here’s the cutdown version of the switch statement. This method finds the row with the contact type and name that have been passed in. The switch statement then decides which field to edit e.g 1 edits the phone number, 2 the Mobile etc. Each case statement will find the textbox using xpath, clear out the field and then enter the new randomly generated contacts details.

The refactoring takes place

First of all I moved out the XPath locators into a dictionary object. That way I can re-use these locators in other methods as well.

public Dictionary<string, string> ContactLocators()
{
    var locatorByXpath = new Dictionary<string, string>
    {
        {"Name", "td[2]/div/Input"},
        {"Telephone", "td[3]/div/Input"},
        {"Mobile", "td[4]/div/Input"},
        {"Email", "td[5]/div/Input"}
     };
     return locatorByXpath;
}

I can access the locators with the 2 lines below

var element = ContactLocators().ElementAt(fieldToEdit);
var locator = element.Value;

The ‘ElementAt’ simply returns the locator at the specified index. The ‘Value’ of the element is the XPath locator itself.

Now I can use this locator to find the textbox on the page

var elementToEdit = row.FindElement(By.XPath(locator));

Next I need to get the new text value from the new contact that was created without using a switch statement. I have a simple one line statement that calls a method to return the  value assigned to any property in an object. In this case I want the value that the new Contact has for the field I’m editing. So if I want the new “Mobile” number to enter into the field, then this method will return the string value assigned to Contacts.Mobile for the new contact.

string newTextToEnter = GetPropertyValueFromObject(element, newContact);

The GetPropertyValueFromObject method itself uses a bit of reflection to retrieve the value assigned to any property

public static string GetPropertyValueFromObject(
KeyValuePair<string, string> element, object T)
{
    var propertyValue = T.GetType().GetProperty(element.Key)
        .GetValue(T, null).ToString();
    return propertyValue;
}

This takes in a KeyValuePair that we retrieved earlier and the object to retrieve the value from (e.g ‘Contacts’ class). Breaking the code down we will get the property based on the element.key e.g. “Name”, “Telephone” etc. and return the value assigned to that property as a string.

Finally we can enter the new value into the field

elementToEdit.ClearAndSendKeys(newTextToEnter);

I have a simple extension method that clears and sends keys to the text-box.

I will further refactor this method, it takes in 4 parameters which is simply too many. I prefer to keep it down to 2 or max 3. That’s next on my list.