My approach to refactoring Paul Stack's monster switch

I read this blogpost a few days ago and was intrigued enough to have a go at refactoring its monster switch statement myself, during the Grand Prix. Here's my approach:

First task, get the creation of a "TaskClass" into a factory. A factory will need an interface to work with, so I defined the simplest one possible:

public interface ITaskClass
{
}

...and implemented it on each taskclass

public class Task1Class : ITaskClass
{
    public void ExtractTop10Data(DateTime dtLastRun)
    {
        return;
    }
}

Next, I created a TaskClassFactory, and duplicated the monster switch into the Create method

public static class TaskClassFactory
{
    public static ITaskClass CreateTaskClass(string name)
    {
        switch (name)
          {
            case "Task1":
                return new Task1Class();
            case "Task2":
                return new Task2Class();
            case "Task3":
                return new Task3Class();
            case "Task4":
                return new Task4Class();
        }
        return null;
    }
}

This gave me the opportunity to write some tests around the creation logic

[TestCase("Task1", typeof(Task1Class))]
[TestCase("Task2", typeof(Task2Class))]
[TestCase("Task3", typeof(Task3Class))]
[TestCase("Task4", typeof(Task4Class))]
public void CreateTest(string name, Type expectedType)
{
    Assert.IsInstanceOf(expectedType, TaskClassFactory.CreateTaskClass(name));
}

Then I introduced the factory into the monster switch (please ignore the smell, this is very much work in progress)

case "Task1":
try
{
    var task1Class = (Task1Class)TaskClassFactory.CreateTaskClass(task.TaskName);
    task1Class.ExtractTop10Data(dtLastRun);
    task.LastRunTime = DateTime.UtcNow;
    UpdateTaskRunStatus(TaskStatus.Good, task);
}
catch (Exception ex)
{
    UpdateTaskRunStatus(TaskStatus.Error, task);
    Logger.LogError("Error details here");
}
break;

Then I consolidated some of the repeated code; first the repeated catch block

try{
    // switch on taskname..
    //There were 22 task names in total making this method over 600 lines long!
    switch (task.TaskName)
    {
        case "Task1":
            var task1Class = (Task1Class)TaskClassFactory.CreateTaskClass(task.TaskName);
            task1Class.ExtractTop10Data(dtLastRun);
            task.LastRunTime = DateTime.UtcNow;
            UpdateTaskRunStatus(TaskStatus.Good, task);
            break;
        case "Task2":
            var task2Class = (Task2Class)TaskClassFactory.CreateTaskClass(task.TaskName);
            task2Class.CreateImageFeed();
            task.LastRunTime = DateTime.UtcNow;
            UpdateTaskRunStatus(TaskStatus.Good, task);
            break;
        case "Task3":
            var task3Class = (Task3Class)TaskClassFactory.CreateTaskClass(task.TaskName);
            task3Class.DestroyImageFeed();
            task.LastRunTime = DateTime.UtcNow;
            UpdateTaskRunStatus(TaskStatus.Good, task);
            break;
        case "Task4":
            var task4Class = (Task4Class)TaskClassFactory.CreateTaskClass(task.TaskName);
            task4Class.ProcessData();
            task.LastRunTime = DateTime.UtcNow;
            UpdateTaskRunStatus(TaskStatus.Good, task);
            break;
        default:
            break;
    }
}
catch (Exception)
{
    UpdateTaskRunStatus(TaskStatus.Error, task);
    Logger.LogError("Error details here");
}

and the common sweep-up tasks

try
{
    // switch on taskname..
    //There were 22 task names in total making this method over 600 lines long!
    switch (task.TaskName)
    {
        case "Task1":
            var task1Class = (Task1Class)TaskClassFactory.CreateTaskClass(task.TaskName);
            task1Class.ExtractTop10Data(dtLastRun);
            break;
        case "Task2":
            var task2Class = (Task2Class)TaskClassFactory.CreateTaskClass(task.TaskName);
            task2Class.CreateImageFeed();
            break;
        case "Task3":
            var task3Class = (Task3Class)TaskClassFactory.CreateTaskClass(task.TaskName);
            task3Class.DestroyImageFeed();
            break;
        case "Task4":
            var task4Class = (Task4Class)TaskClassFactory.CreateTaskClass(task.TaskName);
            task4Class.ProcessData();
            break;
        default:
            break;
    }
    task.LastRunTime = DateTime.UtcNow;
    UpdateTaskRunStatus(TaskStatus.Good, task);
}
catch (Exception)
{
    UpdateTaskRunStatus(TaskStatus.Error, task);
    Logger.LogError("Error details here");
}    

One of the tasks has a parameter to its processing method. I assumed that out of the 22 others not shown there would be more variation to the processing methods. With a nod of acknowledgement to YAGNI, I decided that each task class that needed a parameter to perform its function should be constructed with these parameters; this would mean that I could create a parameterless processing function in the interface. More on this later. Anyway, to enable different constructor parameters to be used in the factory, I created a parameter object and added it to the Create method

public struct Parameters
{
    public DateTime LastRunDate { get; set; }
}
...
public static ITaskClass CreateTaskClass(string name, Parameters createParams)

and in the monster switch:

TaskClassFactory.Parameters factoryParams = new TaskClassFactory.Parameters { LastRunDate = task.LastRunTime };
...
var task1Class = (Task1Class)TaskClassFactory.CreateTaskClass(task.TaskName, factoryParams);

This enabled me to define a parameterless DoWork method on the ITaskClass interface

public interface ITaskClass
{
    void DoWork();
}

This meant I needed to implement the DoWork method in each of the TaskClass implementations. For those with no parameters on the processing method, the implementation was a simple call to that method

public class Task2Class : ITaskClass
{
    public void CreateImageFeed()
    {
        return;
    }
    public void DoWork()
    {
        CreateImageFeed();
    }
}

The one with a parameter needed a little more scaffolding

public class Task1Class : ITaskClass
{
    private DateTime _lastRunDate;
    public Task1Class(DateTime lastRunDate)
    {
        _lastRunDate = lastRunDate;
    }
    public void ExtractTop10Data(DateTime dtLastRun)
    {
        return;
    }
    public void DoWork()
    {
        ExtractTop10Data(_lastRunDate);
    }
}

This forced the factory to change, to use Task1Class's new constructor

public static ITaskClass CreateTaskClass(string name, Parameters createParams)
{
    switch (name)
    {
        case "Task1":
            return new Task1Class(createParams.LastRunDate);

This meant I could hide the existing processing methods in the task classes

private void ExtractTop10Data(DateTime dtLastRun)
{
    return;
}

Next, I updated the monster switch to use the DoWork method for each implementation of ITaskClass

switch (task.TaskName)
{
    case "Task1":
        var task1Class = TaskClassFactory.CreateTaskClass(task.TaskName, factoryParams);
        task1Class.DoWork();
        break;
    case "Task2":
        var task2Class = TaskClassFactory.CreateTaskClass(task.TaskName, factoryParams);
        task2Class.DoWork();
        break;

Then, the money shot: I removed the monster switch itself

public void DoTask()
{
    MySqlConnection oSqlConn = new MySqlConnection();
    List<ScheduledTask> tasksToPerform = oSqlConn.GetTasks();
    try
    {
        foreach (ScheduledTask task in tasksToPerform)
        {
            TaskClassFactory.Parameters factoryParams = new TaskClassFactory.Parameters { LastRunDate = task.LastRunTime };
                
            try
            {
                var taskClass = TaskClassFactory.CreateTaskClass(task.TaskName, factoryParams);
                taskClass.DoWork();                       
                task.LastRunTime = DateTime.UtcNow;
                UpdateTaskRunStatus(TaskStatus.Good, task);
            }
            catch (Exception)
            {
                UpdateTaskRunStatus(TaskStatus.Error, task);
                Logger.LogError("Error details here");
            }
        }
    }
    catch (Exception ex)
    {
        OTLogger.LogGenericError(ex);
    }
    finally
    {
        SQLConnectionHelper.CloseConnection(oSqlConn);
    }
}

 The final code is available as a github:gist
Thanks to CraftyFella for the code formatting