Refactoring Switch statement in my Controller

I'm currently working on a MVC.NET 3 application; I recently attended a course by "Uncle Bob" Martin which has inspired me (shamed me?) into taking a hard look at my current development pr开发者_运维百科actice, particularly my refactoring habits.

So: a number of my routes conform to:

{controller}/{action}/{type}

Where type typically determines the type of ActionResult to be returned, e.g:

public class ExportController
{
    public ActionResult Generate(String type, String parameters)
    {
        switch (type)
        {
            case "csv":
            //do something
            case "html":
            //do something else
            case "json":
            //do yet another thing
        }    
    }
}

Has anyone successfully applied the "replace switch with polymorhism" refactoring to code like this? Is this even a good idea? Would be great to hear your experiences with this kind of refactoring.

Thanks in advance!

The way I am looking at it, this controller action is screaming for a custom action result:

public class MyActionResult : ActionResult
{
    public object Model { get; private set; }

    public MyActionResult(object model)
    {
        if (model == null)
        {
            throw new ArgumentNullException("Haven't you heard of view models???");
        }
        Model = model;
    }

    public override void ExecuteResult(ControllerContext context)
    {
        // TODO: You could also use the context.HttpContext.Request.ContentType
        // instead of this type route parameter
        var typeValue = context.Controller.ValueProvider.GetValue("type");
        var type = typeValue != null ? typeValue.AttemptedValue : null;
        if (type == null)
        {
            throw new ArgumentNullException("Please specify a type");
        }

        var response = context.HttpContext.Response;
        if (string.Equals("json", type, StringComparison.OrdinalIgnoreCase))
        {
            var serializer = new JavaScriptSerializer();
            response.ContentType = "text/json";
            response.Write(serializer.Serialize(Model));
        }
        else if (string.Equals("xml", type, StringComparison.OrdinalIgnoreCase))
        {
            var serializer = new XmlSerializer(Model.GetType());
            response.ContentType = "text/xml";
            serializer.Serialize(response.Output, Model);
        }
        else if (string.Equals("csv", type, StringComparison.OrdinalIgnoreCase))
        {
            // TODO:
        }
        else
        {
            throw new NotImplementedException(
                string.Format(
                    "Sorry but \"{0}\" is not a supported. Try again later", 
                    type
                )
            );
        }
    }
}

and then:

public ActionResult Generate(string parameters)
{
    MyViewModel model = _repository.GetMeTheModel(parameters);
    return new MyActionResult(model);
}

A controller should not care about how to serialize the data. That's not his responsibility. A controller shouldn't be doing any plumbing like this. He should focus on fetching domain models, mapping them to view models and passing those view models to view results.

If you wanted to "replace switch with polymorphism" in this case, you could create three overloaded Generate() ActionResult methods. Using custom model binding, make the Type parameter a strongly-typed enum called DataFormat (or whatever.) Then you'd have:

 public ActionResult Generate(DataFormat.CSV, String parameters)
    {
    }

 public ActionResult Generate(DataFormat.HTML, String parameters)
    {
    }

 public ActionResult Generate(DataFormat.JSON, String parameters)
    {
    }

Once you get to this point, you can refactor further to get the repetition out of your Controller.