Alternative method for Switch Statements

Question

According to some articles:

The theory says that it is easy, but in practice this is often difficult. Never mind. Let's get to the heart of this thing.

The code looks like this:

    public class SomeType
    {
        // Some properties.
    }

    public enum SomeTrigger
    {
        Loaded,
        Initial,
        Timer,
        Final
    }


    public class SomeBaseObject
    {
        // 
        // I am not allowed to change this class. It is not mine.
        //

        protected Dictionary<string, SomeType> Input;
        protected Dictionary<string, SomeType> Output;


        // Timer is evaluated every 2 sec.
        public virtual void Execute(SomeTrigger trigger, string value)
        {
            switch (trigger)
            {
                case SomeTrigger.Loaded:
                    break;
                case SomeTrigger.Initial:
                    break;
                case SomeTrigger.Timer:
                    break;
                case SomeTrigger.Final:
                    break;
                default:
                    break;
            }
        }

        // The rest of code...
    }

And the class that I want to improve:

public class SomeSpecificObject : SomeBaseObject
    {
        private bool isInitializationCompleted;
        private string connection;

        public override void Execute(SomeTrigger trigger, string value)
        {
            switch (trigger)
            {
                case SomeTrigger.Loaded:
                    this.OnLoaded();
                    break;
                case SomeTrigger.Initial:
                    this.OnInitial();
                    break;
                case SomeTrigger.Timer:
                    this.OnTimer();
                    break;
                case SomeTrigger.Final:
                    this.OnFinal(value);
                    break;
                default:
                    break;
            }
        }

        private void OnLoaded()
        {
            // Read Input and Output collection.
        }

        private void OnInitial()
        {
            // Initialization (connect to the server).
            // Bla bla bla
            this.connection = //Value from the plug-in;
            this.isInitializationCompleted = true;

        }

        private void OnTimer()
        {
            if (isInitializationCompleted)
            {
                // Do something
                // Connection is using here.
                // Calculate values on a Input collection, etc.
            }
        }

        private void OnFinal(string value)
        {
            if (isInitializationCompleted)
            {
                // something with "value"
                // Connection is using here.
                // Clear state.
            }
            else
            {
                // Connection is using here.
                // Cancel inistialization
            }
        }
    }

What should I do? Every field is using but every trigger. Additionally the one case is a little specific (OnFinalMethod needs argument). Based on the above articles, I have tried to refactor this code, but unsuccessfully.

My attempts to apply some tips: 

    public interface ITrigger
    {
        void Execute();
    }

    public class LoadedTrigger : ITrigger
    {
        public void Execute()
        {
            throw new NotImplementedException();
        }
    }

    //
    // Exactly the same class for the rest cases.
    //
    public class TriggerHandler
    {
        private Dictionary<SomeTrigger, ITrigger> triggerDictionary;

        public TriggerHandler()
        {

            triggerDictionary.Add(SomeTrigger.Loaded, new InitialTrigger());
    // and for the rest

        }

        public void HandleTrigger(SomeTrigger trigger)
        {
            triggerDictionary[trigger].Execute();
        }
    }

How the objects should communicate with each other? E.g. TimerTrigger objects needs to know that the initiation is successful. What shall I do with the special case object?

Any idea? :)

Its not really clear what problem you are trying to solve, but one design pattern that might work is the [Strategy Pattern](https://sourcemaking.com/design_patterns/strategy). See examples [here](https://stackoverflow.com/a/32415954) and [here](https://stackoverflow.com/a/48809896). — NightOwl888, Feb 16 '18 at 14:33
@NightOwl888 - I've simplified this example. In the real project, the switch statement have 8 cases and the only one class handle all of those behaviors. This class is really big and I though that it is a good idea to delegate some functionalities to a separate classes. — WymyslonyNick, Feb 16 '18 at 15:10
maybe double dispatching by visitor pattern but you need refactor your code then. — underwater ranged weapon, Feb 28 '18 at 12:13

score 1 · Answer 1 · answered Feb 16 '18 at 13:34

At first, there is nothing wrong with the switch statement in general. Just having it in code makes some implicit structural restrictions. If one of these restrictions contradicts with the way how the code evolve, in other words if the requirements changed in a way that switch logic becomes obsolete, then continue having it in the code causes a technical debt.

From the code above it is not clear what are the requirements for it. For example, why there is a requirement to have Execute method for the first place? Can it be replaced with just Loaded(), Initial(), and other methods directly? This obviously will eliminate the switch.

Another approach is to use event objects instead of enums and use method overloading: Execute(LoadedTrigger ...), ..., Execute(FinalTrigger ..., ...).

The approach with dictionary is good if you want to add handlers in runtime (do you?), but the missing part is check for trigger type and calling different execute method:

var t = triggerDictionary[trigger];
if (t is SimpleTrigger st) st.Execute();
else if (t is AdvancedTrigger at) at.Execute(value);

Alternatively, add value to the trigger's execute method. But if there is no such requirement, why to bother?

E.g. TimerTrigger objects needs to know that the initiation is successful.

You probably want a state machine. switch is a frequently used way to code transitions for simple state machines.

score 0 · Answer 2 · answered Mar 01 '18 at 17:37

As one of the approaches you can use template method. With this pattern your code will be looks like this:

public class SomeType
{
    // Some properties.
}

public enum SomeTrigger
{
    Loaded,
    Initial,
    Timer,
    Final
}

public abstract class SomeBaseObject
{
    // 
    // I am not allowed to change this class. It is not mine.
    //

    protected Dictionary<string, SomeType> Input;
    protected Dictionary<string, SomeType> Output;


    // Timer is evaluated every 2 sec.
    public void Execute(SomeTrigger trigger, string value)
    {
        switch (trigger)
        {
            case SomeTrigger.Loaded:
                OnLoaded();
                break;
            case SomeTrigger.Initial:
                OnInitial();
                break;
            case SomeTrigger.Timer:
                OnTimer();
                break;
            case SomeTrigger.Final:
                OnFinal(value);
                break;
            default:
                break;
        }
    }

    protected abstract void OnLoaded();

    protected abstract void OnInitial();

    protected abstract void OnTimer();

    protected abstract void OnFinal(string value);
}

public class SomeSpecificObject : SomeBaseObject
{
    private bool isInitializationCompleted;
    private string connection;



    protected override void OnLoaded()
    {
        // Read Input and Output collection.
    }

    protected override void OnInitial()
    {
        // Initialization (connect to the server).
        // Bla bla bla
        this.connection = "";//Value from the plug-in;
        isInitializationCompleted = true;

    }

    protected override void OnTimer()
    {
        if (isInitializationCompleted)
        {
            // Do something
            // Connection is using here.
            // Calculate values on a Input collection, etc.
        }
    }

    protected override void OnFinal(string value)
    {
        if (isInitializationCompleted)
        {
            // something with "value"
            // Connection is using here.
            // Clear state.
        }
        else
        {
            // Connection is using here.
            // Cancel inistialization
        }
    }
}

If you need, you can provide default implementation of OnXXX methods and mark them as virtual

Alternative method for Switch Statements

2 Answers2