Geek Answers Handbook

Refactoring a switch in my controller

I am currently working on an MVC.NET 3 application; I recently attended the Uncle Bob Martin course, which inspired me (dishonored me?) To carefully study my current development practice, especially my refactoring habits.

So: a number of my routes correspond to:

{controller} / {action} / {type}

Where the type usually determines the type of the returned ActionResult, for example:

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 "swap switch with polymorphism" refactoring to code like this? Is that even a good idea? It would be great to hear your impressions of this kind of refactoring.

Thanks in advance!

+4
source share
2 answers

If you want to “replace the switch with polymorphism” in this case, you can create three overloaded Generate () ActionResult methods. Using custom model bindings, make the Type parameter a strong typed enumeration called DataFormat (or something else). Then you will 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 reorganize further to get a repeat from your controller.

+3
source

As I look at this, this controller action screams for the result of a user action:

 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); }

The controller does not have to worry about how to serialize the data. This is not his responsibility. The controller should not do plumbing like this. He should focus on selecting domain models, matching them to view models, and transferring these viewing models to view results.

+4
source

More articles:


All Articles