8000 Binding against a model class constructor with a single string argument does not work · Issue #1290 · dotnet/command-line-api · GitHub
[go: up one dir, main page]

Skip to content
Binding against a model class constructor with a single string argument does not work #1290
Open
@elgonzo

Description

@elgonzo

UPDATE: There is a resolution for the problem i observed here, and it is with my code. See my follow-up post below. Still, since the unexpected behavior does not really help an unsuspecting user experiencing such problems in discovering the resolution, i decided to keep this issue open for now.

Side note: The example code i used and mentioned here in my report is also on dotnetfiddle https://dotnetfiddle.net/wwlOpl. From what i can tell, the System.CommandLine version 2.0.0-beta1.21216.1 i used on dotnetfiddle behaves in the same way as i describe below.

The documentation explains about binding at https://github.com/dotnet/command-line-api/blob/main/docs/How-To.md#Argument-validation-and-binding:

FileInfo and DirectoryInfo examples of a more general convention whereby any type that has a constructor taking a single string parameter can be bound without having to write any custom code.

and at https://github.com/dotnet/command-line-api/blob/main/docs/model-binding.md#anything-with-a-string-constructor:

But FileInfo and DirectoryInfo are not special cases. Any type having a constructor that takes a single string parameter can be bound in this way. Go back to the previous example and try using a Uri instead.

However, this does not seem to work at all with my own model class(es) with the (as of writing) current code base from GitHub (latest commit being 3264927).

Weirdly, System.FileInfo and System.Uri work (as per documentation), but i can't for the life of me make it work with my own model class...

Given is the following simple model class:

public class MyModel
{
    private readonly string _a;
    private readonly string _b;


    public void OutputMyModelContent()
    {
        Console.WriteLine($"    MyModel._a = {QuoteOrNullWord(_a)}");
        Console.WriteLine($"    MyModel._b = {QuoteOrNullWord(_b)}");
    }


    private static string QuoteOrNullWord(string s) => (s == null) ? "<NULL>" : '"' + s + '"';
}

Note that MyModel has no public properties or fields, so any binding should only happen against constructors that will be added to MyModel in the tests below.

And this is the simple System.CommandLine setup:

var rootCmd = new RootCommand()
{
    new Option<MyModel>("--option")
};
rootCmd.Handler = CommandHandler.Create<MyModel>(
    (m) =>
    {
        Console.WriteLine("Handler invoked with model:");
        m.OutputMyModelContent();
    }
);

rootCmd.Invoke("--option Hello");

In all of the following test scenarios, the commandline arguments will always be --option Hello

Test 1: Lets ignore the documentation and use MyModel with only the default constructor

It doesn't fail with an error or exception. System.CommandLine creates a MyModel instance and passes it to the command handler. The current behavior doesn't seem to be really useful, as there is no and cannot be any information from the commandline passed into the command handler, so what would be the point of allowing this? In my opinion would be better to generate a meaningful exception if a model type does not have properties to bind to nor a constructor with a string argument (but then again, perhaps i am not aware of use cases where the current behavior would make sense).

Test 2: Lets violate the documentation and use MyModel with a constructor having two string arguments

Well, what can i say, i am mean. For this test, lets go completely against the the documentation, and add the following constructor with two string arguments to the MyModel class (with this constructor being the sole constructor of MyModel for this test scenario):

  public MyModel(string a, string b)
  {
      Console.WriteLine($"Constructor MyModel({QuoteOrNullWord(a)}, {QuoteOrNullWord(b)})");
      _a = a;
      _b = b;
  }

Note that the constructor has TWO string arguments. Also note that the constructor writes some console output so it can be observed how and when the constructor is being called.

Running the code now, the following output from the MyModel constructor and command handler will appear:

Constructor MyModel("", "")
Handler invoked with model:
    MyModel._a = ""
    MyModel._b = ""

No error, no exception. Again, a MyModel instance is being created, no information from the commandline is being passed into the MyModel instance, only empty strings. Okay, it is in violation of the documentation, but a better behavior would be to generate a meaningful error or exception if a user is accidentally using a model class like this.

Test 3: Lets keep the two-string constructor from test 2, but also add a constructor with a single string argument

Lets add a single-string constructor to the MyModel class, so that it now has two constructors - one constructor with a single string argument, another constructor with two string arguments (the one from test 2):

  public MyModel(string a)
  {
      Console.WriteLine($"Constructor MyModel({QuoteOrNullWord(a)})");
      _a = a;
      _b = null;
  }

  public MyModel(string a, string b)
  {
      Console.WriteLine($"Constructor MyModel({QuoteOrNullWord(a)}, {QuoteOrNullWord(b)})");
      _a = a;
      _b = b;
  }

This should not be in violation of what has been explained in the documentation and i would expect it working. However, running the code now does not end with success. This is the output:

Constructor MyModel("Hello")
Constructor MyModel("", "")
Handler invoked with model:
    MyModel._a = ""
    MyModel._b = ""

Note how two instances of MyModel are being created. For each of the declared constructors a new MyModel instance is created. The information from the commandline is only provided to the instance that has been created with the single-string constructor. Which i guess would be fine if that MyModel instance would have been passed to the command handler. But, alas, the command handler receives the MyModel instance that has been created using its two-string constructor, missing the information from the commandline.

The expected behavior here is that there should be only one MyModel instance created using the single-string constructor with the information from the commandline. Or, if multiple constructors should not be allowed in a model type, an error/exception should be generated.

Test 4: Lets just have one and only one constructor with a single string argument

What if the MyModel class i changed to possess only one single constructor having only one string argument, without having any other constructors that might confuse System.CommandLine? Lets just remove the two-string constructor and leave the single-string constructor as-is:

  public MyModel(string a)
  {
      Console.WriteLine($"Constructor MyModel({QuoteOrNullWord(a)})");
      _a = a;
      _b = null;
  }

Crossing fingers, running the code, and the output is:

Constructor MyModel("Hello")
Constructor MyModel("")
Handler invoked with model:
    MyModel._a = ""
    MyModel._b = <NULL>

No party today. Again, kind of like in test 3, two instances of MyModel are being created. One with the information from the commandline, one without. And again, like in test 3, the wrong MyModel instance is being passed to the command handler.

Test 5: What if the constructor parameter has to match the name of the option?

(UPDATE: While i tested this as well, i unfortunately forgot to include it in my report. This edit one day later comes a bit late, but better late than never. I am sorry for forgetting to include it.)

Maybe the name of the constructor parameter has to match the option. Lets change the single-string constructor so that the parameter name is "option":

  public MyModel(string option)
  {
      Console.WriteLine($"Constructor MyModel({QuoteOrNullWord(option)})");
      _a = option;
      _b = null;
  }

It did not improve things, unfortunately. Same result as in test 4.

Final test: Lets change Option<MyModel> to Option<string>

Okay, maybe Option<MyModel> should be rather Option<string>? The examples in the documentation using FileInfo and Uri indicate that this is not the correct way to do it, but lets try it anyway:

var rootCmd = new RootCommand()
{
    new Option<string>("--option")
};
rootCmd.Handler = CommandHandler.Create<MyModel>(
    (m) =>
    {
        Console.WriteLine("Handler invoked with model:");
        m.OutputMyModelContent();
    }
);

rootCmd.Invoke("--option Hello");

First, let's try it with the two constructors from test 3:

  public MyModel(string a)
  {
      ...
  }

  public MyModel(string a, string b)
  {
      ...
  }

The output is:

Constructor MyModel("", "")
Handler invoked with model:
    MyModel._a = ""
    MyModel._b = ""

Well, still no good.

Okay, how about using o 5493 nly a single-string constructor like in test 4:

  public MyModel(string a)
  {
      ...
  }

and the output is:

Constructor MyModel("")
Handler invoked with model:
    MyModel._a = ""
    MyModel._b = <NULL>

It seems, there is no winning no matter what i try :-(


P.S.: I suspect the issue of "constructor confusion" goes deeper than this. Issue #1137 illustrates another example of "constructor confusion" in a different set of circumstances. (This suspicion is in all likelihood not true; see my next post.)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      0