Strategy Pattern- Correct Implementation

I'm using this pattern for the first time, and using C#.

I just wanted to check that this is the correct implementation.

I have a button on a Winform, and when clicked on that will output some data in a particular format, defined by choosing from a drop down box. Now this may change in the future, hence me using the Strategy pattern to encapsulate what changes.

I have a 'Strategy Interface' that simply exposes a single method: "Display Data()".

On my button click I use the following code:

private void ConfirmButton_Click(object sender, EventArgs e)
    {
        IViewData viewData;

        switch (outputMedia)
        {
            case "Excel":
                viewData = new ExcelOutput(operation, study);
                viewData.DisplayData();
                break;
            ca开发者_运维知识库se "Spotfire":
                viewData = new SpotfireOutput(operation, study);
                viewData.DisplayData();
                break;
        }


    }

Is this an acceptable way to use this pattern? Obviously if additional output media is identified then I will simply create a new subclass and add an extra 'case' on the switch statement.

Thanks.

The proper way to use Strategy would be to separate the creation of the IViewData object from its usage. The creation itself might be handled by a Factory Method. Then you can use the created IViewData in a separate location, which is totally unaware of the object's concrete class.

E.g.

private IViewData CreateViewData()
{
    IViewData viewData;

    switch (outputMedia)
    {
        case "Excel":
            viewData = new ExcelOutput(operation, study);
            break;
        case "Spotfire":
            viewData = new SpotfireOutput(operation, study);
            break;
    }
    return viewData;
}

...

private void ConfirmButton_Click(object sender, EventArgs e)
{
    IViewData viewData = CreateViewData();

    viewData.DisplayData();
}

Now, from this, you can improve the solution further by refactoring the Factory Method. You may decide to use a Dictionary instead of a switch, or to create the view data objects only once and cache them, or (as you suggested) replacing it with dependency injection ...

First of all you can move viewData.DisplayData(); out of the switch. No need to repeat it for each case. And being able to call DisplayData in the same way for all of them is the whole point you introduced that interface in the first place.

Passing in the parameters in the same way in every case looks a bit repetitive too. So perhaps use a Dictionary<string,Func<Operation,Study,IViewData>> instead. But if different classes require different constructors this isn't going to work.

You should move the switch statement into its own method, ideally on some controller class. This means you dont have to re-write the switch code again when you need an IViewData object.

private void ConfirmButton_Click(object sender, EventArgs e)
{
    IViewData viewData = ViewDataController.GetViewDataController(outputMedia, operation, study);

   viewData.DisplayData();

}

And then in your controller:

 public class ViewDataController
 {
    public static IViewData GetViewDataController(string outputMedia, string operation, string study)
    {
      IViewData viewData = null;

      switch (outputMedia)
      {
        case "Excel":
            viewData = new ExcelOutput(operation, study);
            break;
        case "Spotfire":
            viewData = new SpotfireOutput(operation, study);
            break;
      }
      return viewData;
     }

This will allow you to reuse the same code throughout your application, and if you ever need to add any entries to the switch statement then you only have to do it in one place.