[Mono-dev] Program Option Parsing Library

Jonathan Pryor jonpryor at vt.edu
Thu Jan 10 16:10:07 EST 2008


Thank you for the feedback.

On Thu, 2008-01-10 at 10:43 -0800, Jay Logue wrote:
> Where I think you may have OD'ed on the crack (:-) is in the use of an
> enumerator and ToArray() to parse the arguments.  Its a cleaver
> implementation, and I think you should keep it as an internal
> mechanism.  But from a user perspective it makes the code non-obvious.
> Most people would expect a method called Parse() on an object called
> Options to actually parse the options.  And seeing a call to ToArray()
> where you don't actually use the result (which will be typical for
> commands that don't use extra arguments) is very mysterious.

Agreed.  Options.Parse() should return either a List<string> or a
string[] (probably the latter).  I suspected this was a bad idea when
all of the unit tests needed an extra .ToArray() at the end...

> A better approach, I think, would be have a Parse() method that parses
> the argument list and returns the extra arguments via an out
> parameter.  I like using an out parameter because it signals that
> Parse() is doing more than just producing an array.  

I don't see how an out parameter would be better than a return value,
especially considering that every other .Parse() method in the framework
actually returns a value.

However, a `bool TryParse(string[] args, out string[] result)' might be
useful if the use of exceptions is undesirable.  (However, w/o
exceptions how would the user know what the problem was?)  I'm less sure
about this idea.

> Another thing I think is really important is better error handling for
> typed options.  If the call to TypeConverter.ConvertFromString()
> fails, the exception thrown needs to identify the name of the option
> that failed as well as its value.  I would create a specific exception
> class to convey this (with the original exception inside) so that the
> case can be caught and handled separately in the user's code.

Throwing something other than InvalidOperationException would probably
be good, so that the offending option can be more easily identified.

Thank you for the suggestions.

 - Jon





More information about the Mono-devel-list mailing list