principlecsharpMinor
Nested switches vs domain specific parser
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:
For example:
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:
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;
}
The structure of these commands (wrapped within a normal communication message as defined by this system) is:
- [param] [param] [param]..For example:
sys -messagedelay 123Given 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
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
I think the solution is simplicity: I'd start with the
Or, use a command-line parser library!
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.