Parser.ParseArguments(factory, args) requires parameterless constructor

[ericnewton76]

Issue by Virtlink
Sunday Apr 24, 2016 at 11:12 GMT
Originally opened as gsscoder/commandline#310

---

The Parser.ParseArguments() overloads with a factory function has this signature:

public ParserResult<T> ParseArguments<T>(Func<T> factory, IEnumerable<string> args)
    where T : new()

If I provide my own factory function, why does T need to have a public parameterless constructor (new())?

var result = parser.ParseArguments<Options>(() => new Options(true), args);

And on the other hand, why doesn't the non-factory overload of ParseArguments not require a public parameterless constructor? This will cause a runtime-exception when T doesn't have a public parameterless constructor.

var result = parser.ParseArguments<Options>(args);

[ericnewton76]

Comment by nemec
Monday Apr 25, 2016 at 01:38 GMT

---

I can't speak to the reasoning behind the decision, since I didn't write the code, but they both look like oversights to me. If you want to submit a pull request, I'll merge it.

[ericnewton76]

Comment by Virtlink
Monday Apr 25, 2016 at 10:56 GMT

---

I can fix the first by removing the where T : new(), but I'm not sure about the semantics of the second as adding where T : new() causes tests to fail.

[ericnewton76]

Comment by JeanSebTr
Wednesday Apr 27, 2016 at 18:43 GMT

---

I had the same problem and another fix was necessary (https://github.com/Emergensys/commandline/commit/813766d255b6ad75031655482b8e117fdaec0dc7) to use a factory for a type without parameter less constructor.

I'll check to clean that up and make a pull request (I've also added factory support to multi verb https://github.com/Emergensys/commandline/commit/2ebe156199f8bad3c0155c8c415e3363949941ff), but I'm a bit lost in the functional style.

[ericnewton76]

Comment by CreepyGnome
Wednesday Jun 15, 2016 at 16:50 GMT

---

Doing some digging into the code and unit tests it looks like it is safe to remove the where statement from the ParseArguments that takes a factory Func and it should not be added to the ParseArguments that takes only the args. Basically all the ParseArguments should not have a where statement on them.

Explanation of why I think this:
It will only use the default constructor if the provided type is mutable, and if it is immutable it will use Reflection to figure out the correct constructor to use to create the type. The convention it seems to be expecting is that if the type is mutable then it must have a default constructor, and if it is not expects a constructor that takes the options/args needed to create the type. If you do not follow the expected convention you will get a runtime error, which I think is fine and expected when coding by convention.

If anyone disagrees and thinks it is not safe to remove the one where statement mentioned above let me know, otherwise I will submit a pull request for this issue.