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

Rename or remove static parse method #139

Closed
remkop opened this issue Jun 4, 2017 · 1 comment
Closed

Rename or remove static parse method #139

remkop opened this issue Jun 4, 2017 · 1 comment

Comments

@remkop
Copy link
Owner

remkop commented Jun 4, 2017

Initial change after issue #135 (Method parse signature clash when using from Groovy API) was to rename the instance method from parse to parseCommands, and keep the static parse method unchanged.

I'm now thinking to revisit that decision.

One reason is that the static parse method makes it easy for users to make this mistake:

String[] args = //...
CommandLine cmd = new CommandLine(new App());
cmd.parse(args); // wrong: will interpret first arg as the command to initialize

Another reason is that there are many scenarios that don't involve subcommands where the user wants to call the instance method, not the static method: when type converters are registered, or any other kind of configuration (allow overwritten options, etc, - more of these options to come).

So the instance method may actually be the common use case.

One idea is to keep the static method, but rename it to initialize, and validate that the first argument has @Option or @Parameters annotations.

Another idea is to remove the static parse method altogether.

@remkop
Copy link
Owner Author

remkop commented Jun 4, 2017

Update:

The static method is now called populateCommand and the instance method is renamed to parse.

I hope this naming is more intuitive, but there is no compiler check to prevent users from mistakenly calling the static method without a command object to initialize:

String[] args = //...
CommandLine cmd = new CommandLine(new App());
cmd.populateCommand(args); // wrong: will interpret first arg as the command to initialize

I'm still unsure if the static method should be removed altogether.

For now, I've added validation to throw an IllegalArgumentException if a CommandLine is constructed with an object not annotated with @command, @option or @parameters.

remkop added a commit that referenced this issue Jun 4, 2017
@remkop remkop closed this as completed Jun 7, 2017
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

1 participant