commandline icon indicating copy to clipboard operation
commandline copied to clipboard

I don't think ParseArguments works property with Factory method in VB.NET

Open ClerioVision opened this issue 8 years ago • 7 comments

Whenever I try to to use it this, I get an argument exception with 'factory' as the message.

I'm unable to build the code in VS, so I cant check it out, but I think your implementation of Type.IsMutable does not work properly for a VB class with no settable properties. Maybe there is something else I need to change in my options class to make it immutable?

Here is the options class I am trying

Public Class CommandLineOptions

Public Enum SupressableDevices As Integer
    NI
    GALVO_CHILLER
    LASER_CHILLER
    DOCK_CAMERA
    BACK_CAMERA
    PLATFORM_SENSOR
    OTS
    LASER
    A3200
End Enum

Private ReadOnly m_supressed As IEnumerable(Of SupressableDevices) = Nothing
Private ReadOnly m_verose As Boolean = False

Public Sub New()
    Me.New({}, False)
End Sub
Public Sub New(newSupressed As IEnumerable(Of SupressableDevices), newVerbose As Boolean)
    m_supressed = newSupressed
    m_verose = newVerbose
End Sub

<[Option]("s"c, "Supress", Separator:=":"c, Required:=False, HelpText:="Devices to be supressed.")>
Public ReadOnly Property SupressedDevices As IEnumerable(Of SupressableDevices)
    Get
        SupressedDevices = m_supressed
    End Get
End Property


<[Option]("v"c, "verbose", HelpText:="Print details during execution")>
Public ReadOnly Property Verbose As Boolean
    Get
        Verbose = m_verose
    End Get
End Property

End Class

Why does the options class need to be immutable to use this signature anyway? If I want to use this call to create a mutable instance, why do you prevent it? OK - its not the best practice, I agree. But wouldn't it b e better to allow the code to work rather than throwing an exception?

ClerioVision avatar Jul 24 '17 15:07 ClerioVision

You actually have your terms wrong... you have an Immutable class, but the CommandLineParser library needs the options class to be mutable.

This is because the library uses the type as its mapping from commandline to structured objects, and will never be able to "figure out" how to deal with an immutable options class, for many reasons.

Make your command line options class mutable, so the library can utilize it. If you need immutability for whatever reason, create a factory that knows how to construct an instance of your immutable type from the mutable options type populated by the library.

To fix the problem, make your properties get/set.

ericnewton76 avatar Jul 24 '17 15:07 ericnewton76

Thanks for responding. I am a little confused

  1. I know my class is immutable. I have created a factory method that calls the parameterless constructor and returns the newly created instance, and I reference that that method in my call to ParseArguments.

  2. The call to Parse Arguments always fails - throwing an ArgumentException with 'factory' as the Exception's Message property.

  3. I took a look at the code for the ParseArgument Method with this signature - and I see the following

    public ParserResult<T> ParseArguments<T>(Func<T> factory, IEnumerable<string> args)
        where T : new()
    {
        if (factory == null) throw new ArgumentNullException("factory");
        if (!typeof(T).IsMutable()) throw new ArgumentException("factory");
        if (args == null) throw new ArgumentNullException("args");
    
        return MakeParserResult(
            InstanceBuilder.Build(
                Maybe.Just(factory),
                (arguments, optionSpecs) => Tokenize(arguments, optionSpecs, settings),
                args,
                settings.NameComparer,
                settings.CaseInsensitiveEnumValues,
                settings.ParsingCulture,
                HandleUnknownArguments(settings.IgnoreUnknownArguments)),
            settings);
    

}

It throws the ArgumentException I am seeing if it thinks the type I am trying to create is Mutable. I don't think my type is mutable, but I don't see anyplace else in the code that throws that exception. Unfortunately, I can't seem to build this source in visial studio, so I cant really check it out any more completely. But I think the extension method for Type.IsMutable is returning true for my class, even though it looks to me to be immutable.

Does this make sense?

Does that imply that the type must be immutable?

ClerioVision avatar Jul 24 '17 16:07 ClerioVision

@ericnewton76 is correct, the options class must be mutable as it takes the instance generated by the factory function and sets the appropriate properties.

Did you miss the negation in the parsing method?

if (!typeof(T).IsMutable()) throw new ArgumentException("factory");
    ^

It says if type T is not mutable, throw an exception - therefore you get this exception because your type is immutable, as you state.

So the short answer is you need to create a mutable Options class in order to use the library. Sorry!

nemec avatar Jul 24 '17 16:07 nemec

Aaaahh! Opps I did miss the negation in the code. Thanks

So now I remain confused. In the Wiki for the V2 there is the following section that talks about creating immutable options. Do you understand how I can parse into an immutable class?


Immutable Target

If you develop according to functional programming, you probably won't define a mutable type like the ones presented in samples.

You're free to define an immutable type:

class Options { private readonly IEnumerable files; private readonly bool verbose; private readonly long offset;

public Options(IEnumerable files, bool verbose, long offset) { this.files = files; this.verbose = verbose; this.offset = offset; }

[Option] public IEnumerable Files { get { return files; } }

[Option] public bool Verbose { get { return verbose; } }

[Option] public long Offset { get { return offset; } ] }

The parser will detect this class as immutable by the absence of public property setters and fields.

This is the same feature that allow you to parse against an F# record:

type options = { [<Option>] files : seq; [<Option>] verbose : bool; [<Option>] offset : int64 option; }

As you can see the options.offset record member was defined as option since the library has full support for option<'a> (full CLR name type name Microsoft.FSharp.Core.FSharpOption<T>).

ClerioVision avatar Jul 24 '17 16:07 ClerioVision

Hmm, that part of the code was written by the repo owner and I haven't seen it yet, but maybe you're right that it is possible.

nemec avatar Jul 24 '17 20:07 nemec

@ClerioVision you were so close! First, you cannot use the parse method with the factory (CommandLine will generate its own factory), and second the parameter names in the constructor must match the immutable property names (case insensitive). Once I fixed those, I can parse your options.

Public Sub New(supressedDevices As IEnumerable(Of SupressableDevices), verbose As Boolean)
	m_supressed = supressedDevices
	m_verose = verbose
End Sub

Here's the Main function I used, apologies if it's terrible as I'm not familiar with VB syntax :)

Sub Main
	Dim result = Parser.Default.ParseArguments(Of CommandLineOptions)("-s NI OTS -v".Split())
	result.WithParsed(Function(r As CommandLineOptions)
		Console.WriteLine("Verbose: {0} | Count: {1}", r.Verbose, r.SupressedDevices.Count())
	End Function)
End Sub

nemec avatar Jul 28 '17 22:07 nemec

Wow! Thanks for getting back to me about this - I'd pretty much given up. Did not expect to hear back about this one.

I'll give this a go tomorrow and see what happens.

ClerioVision avatar Jul 29 '17 01:07 ClerioVision