Compact command line argument parser

Question

So, I decided to write my own little command line argument parser for various other projects I work on. I am aware that there are many good command line parser libraries, but I did wrote my own anyway (practice & implementation specific reasons).

The parser works fine, but I have a feeling that it can be improved a lot, mainly the following things come to mind

1. Mainly the actual parser, CommandLineParser.cs. It seems very badly structured and I find it hard to read myself.
2. Abstraction. I wonder if I can abstract it a bit more without making it a pain to use? Maybe by introducing some interfaces?
3. Naming. I went with Option for the command line switch and with Value for the possible parameters. Are my methods/classes self-descriptive?
4. Optimizations. I am sure there are segments that can be done more efficiently, mainly in CommandLineParser.ParseArguments(string[] args)

A couple of things to note:

1. I'd like to keep the structure for the CommandLineValue.cs and CommandLineOption.cs mostly the same as they are part of a plugin architecture to communicate command line arguments between the plugins and the main application.
2. No usage of Attributes to store the command line options.
3. I did write a couple of unit tests to verify the parsers functionality. Despite them being not the main class to review, I am appreciate feedback there too :)

Parser:

public class CommandLineParser { /// <summary> /// Defines all possible command line options the plugin can can process /// </summary> public List<CommandLineOption> SupportedOptions { get; } /// <summary> /// Initialize the commandline parser with a list of commandline options the plugin exposes /// </summary> /// <param name="supportedOptions"></param> public CommandLineParser(List<CommandLineOption> supportedOptions) { SupportedOptions = supportedOptions; } /// <summary> /// Parse the command line arguments and returns a list of commandline values that can be passed to the /// plugin for further processing. The function also handles invalid amount and/or format of options, values /// as well as missing required arguments etc /// </summary> /// <param name="args">The arguments to parse</param> /// <returns>A list of parsed commandline values + options</returns> /// <exception cref="InvalidCommandLineOptionException"></exception> /// <exception cref="InsufficientCommandLineValuesException"></exception> /// <exception cref="InvalidCommandLineValueException"></exception> /// <exception cref="MissingRequiredCommandLineOptionException"></exception> public IEnumerable<CommandLineValue> ParseArguments(string[] args) { var result = new List<CommandLineValue>(); if (args.Length == 0) return Enumerable.Empty<CommandLineValue>(); // Process all command line arguments for (int i = 0; i < args.Length; i++) { CommandLineOption option = null; if (!IsSupportedOption(args[i], out option)) throw new InvalidCommandLineOptionException($"{args[i]} is not a valid command line option"); // Verify if the option expects additional values if (HasAdditionalValues(option)) { // Check if enough additional values are given int additionalValues = option.ParameterTypes.Count; if (i + additionalValues + 1 > args.Length) throw new InsufficientCommandLineValuesException( $"{args[i]} expects {additionalValues} values."); // Check if the additional values are in the right format // ToDo: Find more elegant solution var values = args.ToList().GetRange(i + 1, i + additionalValues).ToList(); var types = option.ParameterTypes.ToList(); var castedValues = values.Zip(types, (value, type) => { try { return Convert.ChangeType(value, type); } catch { throw new InvalidCommandLineValueException( $"Cannot cast between value {value} to type {type}"); } }); result.Add(new CommandLineValue(option, castedValues.ToList())); // Increase i to skip to the next option i += additionalValues; } else { result.Add(new CommandLineValue(option, null)); } } // Collect required arguments List<string> requiredOptions = new List<string>(); foreach (var option in SupportedOptions) { if (option.Required) foreach (var tag in option.Tags) { requiredOptions.Add(tag); } } // Check that no required arguments are missing (or occur twice) var missing = GetMissingRequiredArgs<string>(requiredOptions, args.ToList()); if (missing == null) return result; throw new MissingRequiredCommandLineOptionException( $"The required arument(s) {string.Join(",", missing)} occured multiple times"); } /// <summary> /// Check that all required options are used and that they (the required options) dont occur multiple times are no duplicates /// </summary> /// <param name="required">A list of required options</param> /// <param name="arguments"><The args to check</param> /// <typeparam name="T">Any primitive type</typeparam> /// <exception cref="MissingRequiredCommandLineOptionException">Thrown if any distinct required arguments exist more then once</exception> /// <returns>A list of missing required args, if any. Null if none are missing.</returns> static List<T> GetMissingRequiredArgs<T>(List<T> required, List<T> arguments) { // convert to Dictionary where we store the required item as a key against count for an item var requiredDict = required.ToDictionary(k => k, v => 0); foreach (var item in arguments) { if (!requiredDict.ContainsKey(item)) continue; requiredDict[item]++; // if we have required, adding to count if (requiredDict[item] <= 1) continue; throw new DuplicateRequiredCommandLineOptionException( $"Required option {item} appeared more than once!"); } var result = new List<T>(); // now we are checking for missing items foreach (var key in requiredDict.Keys) { if (requiredDict[key] == 0) { result.Add(key); } } return result.Any() ? result : null; } /// <summary> /// Verify if given option is part of the supported options /// </summary> /// <returns>true if the option is supported otherwise false</returns> private bool IsSupportedOption(string optionIdentifier, out CommandLineOption option) { for (var index = 0; index < SupportedOptions.Count; index++) { var supportedOption = SupportedOptions[index]; if (supportedOption.Tags.Any(tag => tag == optionIdentifier)) { option = SupportedOptions[index]; return true; } } option = null; return false; } /// <summary> /// Indicates if a command line option has multiple values or if its just a flag /// </summary> /// <param name="option">Commandlineoption to check</param> /// <returns>true if the option has multiple values, otherwise false</returns> private bool HasAdditionalValues(CommandLineOption option) { var noParameters = option.ParameterTypes == null || option.ParameterTypes.Count == 0; return !noParameters; } } 

Classes to store commandline information:

public class CommandLineOption { /// <summary> /// The identifier of the commandline option, e.g. -h or --help /// </summary> public ICollection<string> Tags { get; } /// <summary> /// Description of the commandline option /// </summary> public string Description { get; } /// <summary> /// Indicates if the argument is optional or required /// </summary> public bool Required { get; } /// <summary> /// Types of the additional provided values such as directory paths, values etc .. /// </summary> public IList<Type> ParameterTypes { get; } /// <summary> /// Create a new true/false commandline option /// </summary> /// <param name="tags">Identifier of the command line option</param> /// <param name="description">Description of the command line option</param> /// <param name="required">Indicates if the command line option is optional or not</param> public CommandLineOption(IEnumerable<string> tags, string description, bool required = false) { Tags = tags.ToList(); Description = description; Required = required; } /// <summary> /// Create a new true/false commandline option /// </summary> /// <param name="tags">Identifier of the command line option</param> /// <param name="description">Description of the command line option</param> /// <param name="required">Indicates if the command line option is optional or not</param> public CommandLineOption(IEnumerable<string> tags, string description, bool required = false, params Type[] parameterTypes): this(tags, description, required) { ParameterTypes = new List<Type>(parameterTypes); } } 

public class CommandLineValue : IEqualityComparer<CommandLineValue> { /// <summary> /// Holds all the values specified after a command line option /// </summary> public IList<object> Values { get; } /// <summary> /// The command line option the value(s) belong to /// </summary> public CommandLineOption Option { get; set; } /// <summary> /// Stores the values that correspond to a commandline option /// </summary> /// <param name="option">The commandline option the values refer to</param> /// <param name="values">The values that are stored</param> public CommandLineValue(CommandLineOption option, IList<object> values) { Option = option; Values = values; } public bool Equals(CommandLineValue x, CommandLineValue y) { if (x.Option.Description == y.Option.Description && x.Option.Required == y.Option.Required && x.Option.Tags.SequenceEqual(y.Option.Tags) && x.Option.ParameterTypes.SequenceEqual(y.Option.ParameterTypes) && x.Values.SequenceEqual(x.Values)) return true; return false; } public int GetHashCode(CommandLineValue obj) { return base.GetHashCode(); } } 

Custom Exception Classes:

public class DuplicateRequiredCommandLineOptionException : Exception { public DuplicateRequiredCommandLineOptionException(string message) : base(message) { } } public class InsufficientCommandLineValuesException : Exception { public InsufficientCommandLineValuesException(string message) : base(message) { } } public class InvalidCommandLineOptionException : Exception { public InvalidCommandLineOptionException(string message) : base(message) { } } public class InvalidCommandLineValueException : Exception { public InvalidCommandLineValueException(string message) : base(message) { } } public class MissingRequiredCommandLineOptionException : Exception { public MissingRequiredCommandLineOptionException(string message) : base(message) { } } 

Unit Tests:

public class CommandLineParserTests { [Fact] public void ParseDuplicateRequiredArguments() { var args = new[] {"--randomize", "-o", "/home/user/Documents", "--randomize", "-d"}; var supportedOptions = new List<CommandLineOption> { new CommandLineOption( new[] {"-r", "--randomize"}, "Random flag", true), new CommandLineOption( new[] {"-o", "--output-directory"}, "Specifies the output directory", true, typeof(string)), new CommandLineOption( new[] {"-d", "--dummy"}, "Just another unused flag"), }; var parser = new CommandLineParser(supportedOptions); Assert.Throws<DuplicateRequiredCommandLineOptionException>(() => parser.ParseArguments(args) ); } [Fact] public void ParseMissingRequiredArguments() { var args = new[] {"--randomize", "--output-directory", "/home/user/Documents"}; var supportedOptions = new List<CommandLineOption> { new CommandLineOption( new[] {"-r", "--randomize"}, "Random flag"), new CommandLineOption( new[] {"-o", "--output-directory"}, "Specifies the output directory", true, typeof(string)), new CommandLineOption( new[] {"-d", "--dummy"}, "Just another unused flag"), }; var parser = new CommandLineParser(supportedOptions); Assert.Throws<MissingRequiredCommandLineOptionException>(() => parser.ParseArguments(args) ); } [Fact] public void ParseMatchingTypeCommandLineValues() { var args = new[] {"--log", "info", "1337", "3.1415"}; var supportedOptions = new List<CommandLineOption> { new CommandLineOption( new[] {"-l", "--log"}, "Logs info from exactly three data sources", false, typeof(string), typeof(int), typeof(float)) }; var parser = new CommandLineParser(supportedOptions); var expectedValue = new CommandLineValue(new CommandLineOption( new[] {"-l", "--log"}, "Logs info from exactly three data sources", false, typeof(string), typeof(int), typeof(float)), new object[] {"info", 1337, (float) 3.1415}); var actualValue = parser.ParseArguments(args).ToList()[0]; Assert.True(expectedValue.Equals(actualValue, expectedValue)); } [Fact] public void ParseMismatchingTypeCommandLineValues() { var args = new[] {"--log", "info", "1337", "3.1415"}; var supportedOptions = new List<CommandLineOption> { new CommandLineOption( new[] {"-l", "--log"}, "Logs info from exactly three data sources", false, typeof(string), typeof(int), typeof(long)), }; var parser = new CommandLineParser(supportedOptions); Assert.Throws<InvalidCommandLineValueException>(() => parser.ParseArguments(args) ); } [Fact] public void ParseInsufficientCommandLineValues() { var args = new[] {"-l", "info", "info2"}; var supportedOptions = new List<CommandLineOption> { new CommandLineOption( new[] {"-l", "--log"}, "Logs info from exactly three data sources", false, typeof(string), typeof(string), typeof(string)), }; var parser = new CommandLineParser(supportedOptions); Assert.Throws<InsufficientCommandLineValuesException>(() => parser.ParseArguments(args) ); } [Fact] public void ParseInvalidCommandLineOption() { var args = new[] {"--force"}; var supportedOptions = new List<CommandLineOption> { new CommandLineOption(new[] {"-h", "--help"}, "Show the help menu"), }; var parser = new CommandLineParser(supportedOptions); Assert.Throws<InvalidCommandLineOptionException>(() => parser.ParseArguments(args) ); } [Fact] public void ParseNoCommandLineOptions() { var args = new string[] { }; var parser = new CommandLineParser(null); var result = parser.ParseArguments(args); Assert.Equal(Enumerable.Empty<CommandLineValue>(), result); } } 

I appreciate all suggestions. Feel free to be very nitpicky. :)

Answer 1

Design Issues

There are a couple of issues concerning your design.

Lack of specification

It is unclear which features should be supported by your API. This makes reviewing a bit fuzzy.

Dependencies

The parser depends on arguments already pre-parsed correctly by a shell. This limits the control you have over command line parsing.

var args = new[] {"--log", "info", "1337", "3.1415"};

Consider breaking free from the shell and take on pre-parsing yourself.

var args = "--log info 1337 3.1415"; // <- unparsed command line string 

Pollution

The API mixes language structs with user-defined options.

new CommandLineOption(new[] {"-l", "--log"}

You do not want - and -- to be part of the Tags. These are delimiters in the lexing phase of your parser. By seperating lexing from parsing, you could extend the API more fluently by allowing other command line languages. For instance /log.

---

Review

Exception Classes

Define a base class for all your exceptions CommandLineException. This way, you allow calling code to determine the granularity of exception handling. Since you make several custom exceptions, take advantage of storing some data on them. DuplicateRequiredCommandLineOptionException could store the duplicate option, and so on. Also provide constructors that take an inner exception.

public class DuplicateRequiredCommandLineOptionException : CommandLineException { public CommandLineOption Option { get; } // include more constructors .. public DuplicateRequiredCommandLineOptionException( string messageCommandLineOption option) : base(message) { Option = option; } } 

CommandLineOption & CommandLineValue

You have addressed you don't want to see too many changes for legacy reasons. I do propose to override the default Equals and GetHashCode on both classes and substitute IEqualityComparer with IEquatable. This way, you could improve your code.

 public bool Equals(CommandLineValue other) { return Option.Equals(other.Option) && Values.SequenceEqual(other.Values); } 

CommandLineParser

You have indicated yourself you have problems parsing a flattened list to a hierarchical structure. There are common techniques for handling such situations. Have a look at Abstract Syntax Tree. You should create a syntax tree from the provided string[] args. This can be done with a Stack and Iterator. There are tons of examples online how to create an AST.

// Check if the additional values are in the right format // ToDo: Find more elegant solution var values = args.ToList().GetRange(i + 1, i + additionalValues).ToList(); var types = option.ParameterTypes.ToList(); 

The second issue is - what I called pollution before - the lack of seperation of concerns. Your API is basically a simple compiler. The link shows you it's good practice to provide the following phases when building a compiler:

• pre-processing
• lexing
• parsing
• optimizing
• pretty printing

Your API should definitely include lexing and parsing as seperate phases.

• lexing: create command line tokens and strip all the keywords and language-specific delimiters
• parsing: create an AST from the lexed tokens, then create CommandLineValue instances from the AST.

---

Conclusion

In the end, the quality of the API depends on a good specification covered by many unit tests. I feel you haven't established this yet.