Strategy Pattern- Correct Implementation

Question

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;
            case "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.

Answer 1

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 ...

Answer 2

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.