HiveBrain v1.2.0
Get Started
← Back to all entries
principlecsharpMinor

Nested switches vs domain specific parser

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
parserswitchesnestedspecificdomain

Problem

I'm working on an application which has a defined and immutable (for our purposes) communication protocol. One of the features is that users on the controlling terminal can enter text commands that in many ways mimic interacting with a commandline application.

The structure of these commands (wrapped within a normal communication message as defined by this system) is:

 - [param] [param] [param]..


For example:

sys -messagedelay 123


Given that the commands act on the behavior of the communication manager itself (the internals of which I don't want to expose outside of that class) which of the following is the better approach?

Using a nested switch:

private SendReceiveResult HandleCommand(string command)
{
    string[] splitCommand = command.Split(CommandSplitChars);
    switch (splitCommand[0])
    {
        case "sys":
            if (splitCommand.Length < 2)
            {
                HandleError("Invalid sys command");
                break;
            }
            switch (splitCommand[1])
            {
                case "-messagedelay":
                    if (splitCommand.Length < 2 ||
                        !float.TryParse(splitCommand[1], out messagedelay))
                    {
                        HandleError("Missing or invalid messagedelay parameter");
                    }
                break;
                // .. more cases
            }
            break;
            // .. more cases
    }
}


Using a private parser class, with delegates per command:

```
// Private nested command parser class
private class CommandParser : IEqualityComparer
{
private Dictionary> commandHandlers;
private Func defaultHandler;

public CommandParser(Func defaultHandler)
{
commandHandlers = new Dictionary>(this)
this.defaultHandler = defaultHandler;
}

public void AddCommandHandler(Func commandHandler, params string[] pattern)
{
commandHandlers[pattern] = commandHandler;
}

Solution

Nested Switch

Nasty smell, but not such a bad idea - if it gets it done, then it could easily be refactored into a much better-looking and less error-prone form. There are already tons of posts here and on StackOverflow about refactoring switch blocks, the common way would be to use a Dictionary and map each key to a method.


Parser Class / Delegates per Command

Another, different smell, but still smelly: over-complicated. [K]eep [I]t [S]imple, [S]tupid. This is definitely shredding KISS into nano-pieces, and I have yet to understand why you would want a parser class to implement IEqualityComparer (merely just glanced at the code).

I think the solution is simplicity: I'd start with the switch blocks, get the logic together, and refactor until satisfaction is achieved (I'd maybe end up extracting classes for each command, extracting interfaces for everything they all have in common, etc).

Or, use a command-line parser library!

Context

StackExchange Code Review Q#36144, answer score: 4

Revisions (0)

No revisions yet.