Nested switch? Or other solution checking different conditions?

Question

Okay, the situation is as following:

I have one client that has 2 settings: ConnectionState and ConnectionSollState, both the same enumerable (TypeConnectionState), they store the actual state of the client connection and the state the connection should be. On every combination of the to, something different should happen, like when ConnectionState is "Connected" but ConnectionSollState is "Closed" -> Teardown the client. So I have like 4 possibilities that I have to check. Now every client can handle an infinite number of sessions, and every session has also a state (StreamState & StreamSollState), those states can have 6 options in enumerable.

From now on, I'm making like 20 switch-conditions and my code looks really messy, I'm doing something wrong every 5 minutes while coding. Is there an easier way to handle a situation like this? (if/else) would make things waaay worse.

Example:

private void RTSPWorker() {
    try {
        byte[] buffer = new byte[2048];
        while (!mb_RTSPWorkerAbbort) {
        // Call TransportWD
            Thread.Sleep(100 * mi_ConnectionTimeOut);

            // Check ConnectionSollState
            switch(ConnectionSollState) {
                case TypeConnectionState.Connected:
                    // ConnectionSollState = Connected, check ConnectionState
                    switch(ConnectionState) {
                        case TypeConnectionState.Connected:
                            // ConnectionState is connected, keep-alive!
                            if(GET_PARAMETER() == null) {
                                DESCRIBE();
                            }
                            // Check streams too
                            foreach (cRTSPStream oStream in mo_StreamDict.Values) {
                                // Check StreamSollState
                                switch(oStream.RTSPStreamSollState) {
                                    case cRTSPStream.TypeRTSPStreamState.Play:
                                        // SollState is PLAY, check State
                                        switch(oStream.RTSPStreamState) {
                                            case cRTSPStream.TypeRTSPStreamState.Play:
                                                //Stream is alive, keep-alive!
                                                if (oStream.PLAY() == null) { oStream.DESCRIBE(); } break;
                                            case cRTSPStream.TypeRTSPStreamState.Closed:
                                                // Reinitialise.
                                                if (oStream.SETUP() != null) { oStream.PLAY(); } break;
                                            default:
                                                // Default, send play.
                                                oStream.PLAY(); break;
                                        }
                                    break;
                                case cRTSPStream.TypeRTSPStreamState.Pause:
                                    // SollState is on pause, check State
                                    switch(oStream.RTSPStreamState) {
                                        case cRTSPStream.TypeRTSPStreamState.Closed:
                                            // Reinitialise.
                                            if (oStream.SETUP() != null) { oStream.PLAY(); } break;
                                        default:
                                            oStream.PAUSE();
                                            break;
                                    }
                                break;
                                case cRTSPStream.TypeRTSPStreamState.Closed:
                                    // SollState is closed, check State
                                    switch(oStream.RTSPStreamState) {
                                        case cRTSPStream.TypeRTSPStreamState.Closed:
                                            // Is closed, do nothing
                                            break;
                                        default:
                                            // Default teardown, remove session
                                            oStream.TEARDOWN();
                                            this.RemoveRTSPSession(oStream);
                                            break;
                                        }
                                default:
                                    // Default, what do?
                                    break;
                                }
                            }
                            break;
                        case TypeConnectionState.Closed:
                            // ConnectionState should be connected, re-connect!
                                while(Connect() != true) {
                                    // Sleep for 200ms, try again
                                    Thread.Sleep(200);
                                }
                            break;
                        default:
                            // TODO anything else
                            break;
                    }
                    break;
                case TypeConnectionState.Closed:
                    // Check ConnectionState
                    switch(ConnectionState) {
                        case TypeConnectionState.Connected:
                            // Is connected, should be closed. Close connection & clean up!
                            Close(null);
                            break;
                        default:
                            // Anything other than Connected, do nothing.
                            break;
                    }
                    break;

                default:
                    break;
            }
        }
    } catch {

    }
}

Why don't you move certain things into smaller functions? That would clear things up. — Jensen, Sep 04 '12 at 13:06
In the cases where you have only the Connected check in the switch statement you should use if-statements. Too much boilerplate code otherwise. — BlueM, Sep 04 '12 at 13:06

Martin Devillers · Accepted Answer · 2012-09-04T15:05:14.203

Holy smokes, Batman, that is some incomprehensible code you have there. Here are some tips that might improve your situation. I must add that these tips are somewhat biased as they prefer a personal coding style:

Rewrite the switch-statements into IF-statements. Nested IF-statements are much easier to read than nested switch-statements, even though you will have to write a bit more code to get to the same result.
Break your method into several sub-methods. Your method is probably trying to do too many things at the same time. -> Seperation of Concerns & Single Responsibility Principle
Use improved truth-tables. In situations where you have logic that produces some outcome based on combinations of variables, you may want to use a truth-table construction. For instance, using a List<Tuple<T1,T2,T etc..>> construction.

Ex:

var truthTable = List<Tuple<NavigationTab, Role, Action>>
{
    new Tuple<NavigationTab, Role, Action>(NavigationTab.Users, Role.UsersAdministration, MVC.Administration.Users.Index()),
    new Tuple<NavigationTab, Role, Action>(NavigationTab.Users, Role.RolesAdministration, MVC.Administration.Roles.Index()),
    new Tuple<NavigationTab, Role, Action>(NavigationTab.Products, Role.ProductsAdministration, MVC.Administration.Products.Index()),
}

I was under the influence that if statements are slower than switch statements, meaning that in cases where you are nesting you could be losing quite a bit of performance (depending on how complex the switch / if is and how many nested levels there are of course) — jgok222, Aug 13 '13 at 14:39
Quite right, but that's a performance difference in the order of nanoseconds. In 99.9% of all real-life scenario's this level of performance difference is absolutely negligible. Instead, matters like code legibility and maintainability are much more important. — Martin Devillers, Aug 13 '13 at 17:57

score 1 · Answer 2 · answered Sep 04 '12 at 13:18

Better object orientation is generally considered the best alternative to switch statements. For instance, you can use polymorphism instead of switch statements and do the following:

try {
    byte[] buffer = new byte[2048];
    while (!mb_RTSPWorkerAbbort) {
        Thread.Sleep(100 * mi_ConnectionTimeOut);
        worker.DoWork();
    }
 } catch (Exception ex) {
     Trace.WriteLine(ex);
 }

As state changes you can instantiate different types of worker classes that implement DoWork differently. One might be:

while(Connect() != true) {
    // Sleep for 200ms, try again
    Thread.Sleep(200);
}

Of course, I don't necessarily think you should do it exactly that way. The only thing I have to go by is the code you posted--so, there's likely much better ways of implementing different behaviour.

This would be very similar to the Specification Pattern.

score 0 · Answer 3 · answered Sep 04 '12 at 14:03

Break your code into smaller, more manageable pieces.

If you extract the logic in your case statements, the main loop could be rewritten without a switch at all, such as:

while (!mb_RTSPWorkerAbbort)
{
    // Call TransportWD
    Thread.Sleep(100*mi_ConnectionTimeOut);

    if (ConnectionSollState == TypeConnectionState.Closed)
    {
        CloseIfConnected();
    }

    if (ConnectionSollState == TypeConnectionState.Connected)
    {
        if (ConnectionState == TypeConnectionState.Connected)
        {
            KeepAlive();
            ProcessStreams(); // <--bad example name - rename to what it's actually doing.
        }
        else
        {
            Reconnect();
        }
    }
}

When you see this a month from now (or in case like this when you need a review) it will be a lot easier to see what is going on.

Nested switch? Or other solution checking different conditions?

3 Answers3