jcommander

jcommander copied to clipboard

Usage of output parameters (i.e. sending an instance of the "Parameter descriptor" class which is modified in place by JCommander.parse(...)) has proven to be susceptible to programming errors. It's also not in line with a more functional style of programming.

I suggest to introduce some convenience methods that do not use output parameters. Instead, they should rely only on getting a java.lang.Class object describing the parameters, and not an instance of the "Parameter descriptor". JCommander could make sure to create a "Parameter descriptor" instance via reflection.

Params myParams = JCommander.parse(Params.class, myArgs);
String usage = JCommander.usage(Params.class, myProgramName);

This issue relates to Issue #127

~~I intend to create a pull request with this feature as soon as possible.~~ This can be resolved by accepting pull request: https://github.com/cbeust/jcommander/pull/172

Thanks,

What's the point in this? This approach is less flexible than the current one since callers very often want to instantiate that object themselves so they can pass in additional dependencies. It also introduces unnecessary additional reflection, which I'd rather avoid, and prevents callers from using dependency injection for their parameters.

What's the point in this? This approach is less flexible than the current one since callers very often want to instantiate that object themselves so they can pass in additional dependencies. It also introduces unnecessary additional reflection, which I'd rather avoid, and prevents callers from using dependency injection for their parameters.

Hi, and thank you for such a quick reply, it's much appreciated!

The point would be to make the API usage easier and more obvious for a specific subset of use cases. The convenience methods introduced are not meant to replace the existing fine grained methods - instead, if you look at the pull request referenced here, the convenience methods are intended to use the existing fine grained methods under the hood. In this implementation, the convenience methods become a shorthand for performing the most basic use cases - and getting them right every time.

I'll admit that I'm as little allergic to output parameters, because they rely on mutable state. For instance, if a novice/tired developer was to create an output parameter instance on instantiation of the calling class (e.g. some kind of MyShellCommand implementation) for later reuse, the implementation is suddenly dependent on things like in which order you call methods on the MyShellCommand, if you re-instantiate it between calls, etc. Of course, you can address these issues in a couple of different ways:

• Just do it right (TM) - I think we all know how this one usually turns out :)
• Always reinstantiate the output parameters on use! - This risks introducing a lot of boilerplate code, if done wrong
• Use convenience API methods that do not rely on mutable state

Of course, as you point out, the convenience methods will never be the right thing to use for every use case, but should prove useful for that specific subset of simple use cases that are (in my opinion) overly complicated in the current API.

On Thu, Nov 21, 2013 at 7:29 AM, magnusvojbacke [email protected]:

What's the point in this? This approach is less flexible than the current one since callers very often want to instantiate that object themselves so they can pass in additional dependencies. It also introduces unnecessary additional reflection, which I'd rather avoid, and prevents callers from using dependency injection for their parameters.

Hi, and thank you for such a quick reply, it's much appreciated!

The point would be to make the API usage easier and more obvious for a specific subset of use cases. The convenience methods introduced are not meant to replace the existing fine grained methods - instead, if you look at the pull request referenced here, the convenience methods are intended to use the existing fine grained methods under the hood. In this implementation, the convenience methods become a shorthand for performing the most basic use cases - and getting them right every time.

I'll admit that I'm as little allergic to output parameters, because they rely on mutable state.

Not sure what you mean, your argument suggestion (instantiated by JCommander) is just as mutable as the way it's implemented right now (instantiated by the caller). These arguments are set via reflection, so while they are mutable, that mutability is invisible to the called anyway.

Either way, my objections still stand: your proposal decreases the flexibility offered by the current approach, where a lot of users use dependency injection to share the command line args across their code base, and it adds more reflection, which means more risks of runtime error (e.g. if the user forgets to specify a no-arg constructor for their argument class).

Cédric

Let me summarize my objection curtly:

I see zero advantages to

JCommander.parse(args, Params.class);

over

Params a = new Params();
new JCommander(args).parse(a);

Actually, I see nothing but drawbacks.

I'm going to reiterate my point, because adding these convenience methods will not decrease the flexibility of the current API. The convenience methods introduced are not meant to replace the existing flexible methods - they are only meant to add to the existing API.

I understand that, but this API doesn't add any new functionality and can be emulated with a five line helper method...

I see zero advantages to

JCommander.parse(args, Params.class);

over

Params a = new Params(); new JCommander(args).parse(a);

I disagree. I claim that there are advantages in the readability of the code you have to write to use it.

Params scanned = JCommander.parse(args, Params.class)

Pretend you are a programmer without knowledge of the JCommander API who finds this line of code inside a larger project. When you read the line above, you really only have to scan it once to guess its' intentions, side effects and result: "Aha, we use something called JCommander to parse the 'args' array, and the result is returned and saved in the new variable 'scanned'"

If we pretend that the same programmer finds this instead:

Params a = new Params();
new JCommander(args).parse(a);

You will probably have to give it two or more reads to determine its' intentions: "We use something called JCommander to parse the a array... But wait, where does the result go? Oh, now I see - we save the reference to the 'args' object. I guess that means 'args' is an output parameter that is mutated in place by the call to parse."

What if you have multiple argument classes?

In the case of multiple argument classes, I guess you just fall back to the original way of using the JCommander API. It would be one of the non-trivial use cases where the convenience methods won't be a good fit.

Hey @cbeust , I agree with @magnusvojbacke. This option is a good to have: JCommander.parse(args, Params.class)