1

I am working with C# and trying to create a way to parse a file in which every line is a new command for a program I have.

My file looks something like this:

  • move 10 20
  • stop 1000
  • move 20 24

Right now I only have two possible instructions, Move and Stop, and they have different parameters.

I am trying to future proof my code and make it as generic as possible because I will eventually add more commands in the future.

So I decided to have an abstract class Command that implements the method Update() and then the commands inherit from Command and specify what Update does.

This commands end up in a List and I call update for each element in the list.

(This is probably not the best implementation and feel free to propose better ones)

Now I have to read the file and parse this commands, this means creating new Move or Stop objects in runtime, but the only way I have to know which one I have to create is when I read the file line.

I know I could do it with a simple switch statement for the first word but that makes me having to add by hand a new case block every time I had a new command.

Is there a better way to instantiate objects of a given subclass in runtime other then going through a switch for every possible option ?

Thanks in advance!

  • You could try using Reflection, see this answer: http://stackoverflow.com/a/15452879/1220550 – Peter B Dec 15 '16 at 12:01
  • @PeterB That was fast! Thank you very much! I have always eared that reflection is bad and slow and that one should use generics, Is this doable with generics ? That's somewhat uncharted territory for me. – Alexandre Laborde Dec 15 '16 at 12:07
  • Be warned....`Activator.CreateInstance()` is several magnitudes **slower** than say `new Move()` –  Dec 15 '16 at 12:18

1 Answers1

1

In your simple scenario where the syntax will always be [Command] [Space separated numeric argument list] you could consider the following approach:

Implement a Dictionary<string, Func<IEnumerable<int>, Command>> where the key would be the command's reserved word and the value a delegate that would take the parsed string arguments (I've taken the liberty of assuming they will all be integers) and build the appropiate command. These delegates could point to static factory methods implemented in Command similar to how Linq.Expressions is built:

public abstract class Command
{
     private const int validMoveArgumentCount = 2;
     private const int validStopArgumentCount = 1;

     public static Command Move(IEnumerable<int> args)
     {
         if (args == null)
             throw new ArgumentNullException(nameof(args));

         var arguments = args.ToArray();
         var argumentCount = arguments.Length;

         if (argumentCount != validMoveArgumentCount)
             throw new SyntaxErrorException("Invalid number of arguments in Move command. Expected {validMoveArgumentCount}, received {argumentCount}.");

         return new MoveCommand(arguments[0], arguments[1]);
     }

     public static Command Stop(IEnumerable<int> args)
     {
         if (args == null)
             throw new ArgumentNullException(nameof(args));

         var arguments = args.ToArray();
         var argumentCount = arguments.Length;

         if (argumentCount != validStopArgumentCount)
             throw new SyntaxErrorException("Invalid number of arguments in Stop command. Expected {validStopArgumentCount}, received {argumentCount}.");

         return new StopCommand(arguments[0]);
     }

     public abstract void Update();
     ...

     private class MoveCommand: Command { ... }
     private class StopCommand: Command { ... }
}

Do note a pattern I really enjoy a lot; nested private classes that derive from the containing abstract class. This is a neat way of completely hiding the implementation of the concrete commands and controlling precisely who can instantiate them.

You'd need to have a initialization method where you build up the dictionary, something along the lines:

var commandDictionary = new Dictionary<string, Func<IEnumerable<int>, Command>>();
commandDictionary["move"] = args => Command.Move(args);
commandDictionary["stop"] = args => Command.Stop(args);

This is essentially where you wire up your parsing logic.

And now when parsing each line of your commands input, you'd do something similar to (oversimplifying of couse):

private Command ParseCommand(string commandLine)
{
    Debug.Assert(!string.IsNullOrWhiteSpace(commandLine));

    var words = commandLine.Split(new[] { ' ' }, StringSplitOptions.RemoveEmptyEntries);

    if (commandDictionary.ContainsKey(words[0]))
        return commandDictionary[words[0]](words.Skip(1).ParseIntegerArguments());

    throw new SyntaxErrorException($"Unrecognized command '{words[0]}'.");
}

private static IEnumerable<int> ParseIntegerArguments(IEnumerable<string> args)
{
     Debug.Assert(args != null);

     foreach (var arg in args)
     {
         int parsedArgument;

         if (!int.TryParse(arg, out parsedArgument))
             throw new SyntaxErrorException("Invalid argument '{arg}'");

         yield return parsedArgument;
     }
}

All that said, here you are not using a switch statement but you are building up a dictionary. At the end of the day, you somehow have to define what commands are valid and how you are going to process them. Maybe with this approach, adding new commands is a little cleaner, but thats up to personal taste.

Also worth mentioning that, for simplicity's sake, I'm throwing exceptions when encountering incorrect syntax. This wouldn't be the way I'd do it if I were seriously implementing a similar parser because, as Eric Lippert would put it, I'd be throwing particularly vexing exceptions. I'd probably just parse the whole thing as best as possible (error recovery is pretty trivial in this particular case) adding descriptive error objects to a diagnosticsBag (some IList<ParsingError>) when appropiate and then producing an aggregated parsing error report.

InBetween
  • 32,319
  • 3
  • 50
  • 90
  • Exactly. Well said –  Dec 15 '16 at 12:23
  • This solution is just amazing! I will try it out right now. Thank you very much. The ideia here would be to build my command list using the `ParseCommand` function. Does that mean that my implementation where I put all commands in a list and iterate it is acceptable?. Once again thank you very much. – Alexandre Laborde Dec 15 '16 at 13:28
  • @AlexandreLaborde You are welcome! Yes, you'd build your command list using `ParseCommand` and then simply iterate over it executing the commands in order. You'd probably want to use a `Queue` in this case to explicitly declare that execution order is important. – InBetween Dec 15 '16 at 13:40
  • @InBetween will do! Thank you very much. – Alexandre Laborde Dec 15 '16 at 13:57