Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set optional parameters to empty container if type is array or collection #286

Closed
JanMosigItemis opened this issue Feb 12, 2018 · 6 comments

Comments

@JanMosigItemis
Copy link

JanMosigItemis commented Feb 12, 2018

Hi there,

I recently had the following situation:

@Parameters(arity = "0..*", description = "tbd") private Set<Type> someParams;

If I do not specify any params at command line, someParams is null which is a bit surprising / annoying when it comes to working with arrays / collections.

The Documentation says:

If the field is null, picocli will instantiate it.

Which does not seem to be the case if no params are specified.

Is this the default picocli behavior? Do I miss something?

I would like picocli to create an empty array / collection in this case. This helps to keep the code simple and comprehensible as checking for null on container members is odd and causes boiler plate.

Thank you very much for any input on this.

@remkop
Copy link
Owner

remkop commented Feb 12, 2018

You are correct that this is the current behaviour: fields are only instantiated if picocli finds a matching argument on the command line.

For single-value fields, this allows applications to detect which options and parameters were specified on the command line, and which ones were not. For consistency I kept the same behaviour with multi-value fields (arrays and collections).

Overall, picocli can be improved in the area of assigning default values. I recently started thinking about this more after discussion in issue #261. It seems to me that your request falls into the category of "picocli needs better defaulting", so is related to #261. I can easily imagine a IDefaultValueProvider implementation that instantiates multi-value fields...

Question though, if the main concern is avoiding null-checking, why not simply instantiate the field with the declaration, like this:

@Parameters(arity = "0..*", description = "tbd")
private Set<Type> someParams = new LinkedHashSet<>();

@JanMosigItemis
Copy link
Author

I didn't know that this was possible because I do not know when picocli is going to set the command's members. So, I understand that I could specify "default values" directly as plain old Java member initializer? Picocli will not touch them if it finds no match? This would be acceptable for me (may be a good point to include into the documentation though).

FYI: My whole point is somewhat influenced by Josh Bloch's Effective Java Item 54: Return empty collections or arrays, not nulls.

@remkop
Copy link
Owner

remkop commented Feb 12, 2018

Yes, that is correct. Field initializers are currently the only way to provide default values.
(For arrays and collections, there is one "gotcha": if you specify a non-null array or collection, picocli will add values to this array or collection. Existing values are not replaced at the moment. This makes it impossible to specify a default other than an empty array or collection... There is an outstanding request to improve this: #216)

Thanks for pointing out that the documentation for this can be improved, I'll take a look!

@JanMosigItemis
Copy link
Author

JanMosigItemis commented Feb 12, 2018

I like! Thx for the quick answers.

@remkop
Copy link
Owner

remkop commented Feb 12, 2018

Glad to hear that solved the issue. Are you okay to close this ticket?

remkop added a commit that referenced this issue Feb 13, 2018
@remkop remkop added this to the 2.3 milestone Feb 13, 2018
@remkop remkop closed this as completed in d62cfad Feb 13, 2018
@remkop
Copy link
Owner

remkop commented Feb 13, 2018

The user manual has been updated with better docs for the 2.3 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants