[jira] [Commented] (CLI-284) Inconsistency in constraints for creating an instance of class Option using builder pattern or constructor

2020-06-16 Thread chenshuming (Jira)


[ 
https://issues.apache.org/jira/browse/CLI-284?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17138011#comment-17138011
 ] 

chenshuming commented on CLI-284:
-

 [~fury]

+1, agree.

Option instance can have longOpt value null first and set real value later.

The check should in Options.addOption(Option) because it means we think Option 
instance is set up ready.

But if we add a check in Options.addOption(Option) , we want to throw a 
exception when check fail. That may break the binary compatibility.

> Inconsistency in constraints for creating an instance of class Option using 
> builder pattern or constructor
> --
>
> Key: CLI-284
> URL: https://issues.apache.org/jira/browse/CLI-284
> Project: Commons CLI
>  Issue Type: Bug
>Reporter: Dilraj Singh
>Assignee: Bruno P. Kinoshita
>Priority: Major
>
> Builder pattern for creating an instance of class *Option* ensures that at 
> least one of the representation (short and long representation) for the 
> option is not null. It throws an *IllegalArgumentException* in-case program 
> tries to build an instance with both the representations as null. Consider 
> the following code snippet for reference.
> {code:java}
> public static void main(String[] args) {
>   Option.Builder builder = Option.builder();
>   Option op = builder.build();
> } 
> {code}
> This piece of code fails with the following exception
> {noformat}
> Exception in thread "main" java.lang.IllegalArgumentException: Either opt or 
> longOpt must be specified
> {noformat}
> But if we try to create an instance of Option by calling its constructor, It 
> allows the creation with both opt and longOpt for Option as null. Consider 
> the following code snippet for reference
> {code:java}
> public static void main(String[] args) {
>   Option op = new Option(null, null);
>   System.out.format("Short Representation: %s\n" +
>   "Long Representation: %s", op.getOpt(), 
>   op.getLongOpt());
> }
> {code}
> Output:
> {noformat}
> Short Representation: null
> Long Representation: null
> {noformat}
> Calling a method on an instance with both opt and longOpt as null will lead 
> to undesired null pointer exception. For example, calling
> {code:java}
>  op.getId() {code}
> will throw a null pointer exception, thus violating the method contract.
> So as to fix this we need to make sure that whenever a constructor is invoked 
> it has a non-null argument value for at least one of the Option 
> representation. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CLI-284) Inconsistency in constraints for creating an instance of class Option using builder pattern or constructor

2018-06-10 Thread Stephan Fuhrmann (JIRA)


[ 
https://issues.apache.org/jira/browse/CLI-284?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16507449#comment-16507449
 ] 

Stephan Fuhrmann commented on CLI-284:
--

I've been thinking about the problem a little. The problem is that there's the 
{{Option.setLongOpt()}} method. This means you can in any time in the future 
set the long option. You can also set it to {{null}}, rendering all previous 
checks {{(opt != null || longOpt != null)}} at construction time senseless.

The design of the {{Option}} class is the problem. It's not _immutable_, but 
has two different builders interacting with it and you can build an {{Option}} 
instance also yourself. 

{{Options.addOption(Option)}} is the final place where you could do a 
validation. But this would be _breaking the API_ because {{addOption(Option)}} 
is not documented to throw an exception.

The only way I see is to _break the API_ by removing the {{setLongOpt()}} 
method, extending the constructors to all have the long-version also (nullable).

I think this doesn't make sense for Commons CLI 1 and propose to _won't fix_ 
this bug. Any other opinions?

> Inconsistency in constraints for creating an instance of class Option using 
> builder pattern or constructor
> --
>
> Key: CLI-284
> URL: https://issues.apache.org/jira/browse/CLI-284
> Project: Commons CLI
>  Issue Type: Bug
>Reporter: Dilraj Singh
>Assignee: Bruno P. Kinoshita
>Priority: Major
>
> Builder pattern for creating an instance of class *Option* ensures that at 
> least one of the representation (short and long representation) for the 
> option is not null. It throws an *IllegalArgumentException* in-case program 
> tries to build an instance with both the representations as null. Consider 
> the following code snippet for reference.
> {code:java}
> public static void main(String[] args) {
>   Option.Builder builder = Option.builder();
>   Option op = builder.build();
> } 
> {code}
> This piece of code fails with the following exception
> {noformat}
> Exception in thread "main" java.lang.IllegalArgumentException: Either opt or 
> longOpt must be specified
> {noformat}
> But if we try to create an instance of Option by calling its constructor, It 
> allows the creation with both opt and longOpt for Option as null. Consider 
> the following code snippet for reference
> {code:java}
> public static void main(String[] args) {
>   Option op = new Option(null, null);
>   System.out.format("Short Representation: %s\n" +
>   "Long Representation: %s", op.getOpt(), 
>   op.getLongOpt());
> }
> {code}
> Output:
> {noformat}
> Short Representation: null
> Long Representation: null
> {noformat}
> Calling a method on an instance with both opt and longOpt as null will lead 
> to undesired null pointer exception. For example, calling
> {code:java}
>  op.getId() {code}
> will throw a null pointer exception, thus violating the method contract.
> So as to fix this we need to make sure that whenever a constructor is invoked 
> it has a non-null argument value for at least one of the Option 
> representation. 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CLI-284) Inconsistency in constraints for creating an instance of class Option using builder pattern or constructor

2018-06-10 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/CLI-284?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16507370#comment-16507370
 ] 

ASF GitHub Bot commented on CLI-284:


Github user sfuhrm closed the pull request at:

https://github.com/apache/commons-cli/pull/25


> Inconsistency in constraints for creating an instance of class Option using 
> builder pattern or constructor
> --
>
> Key: CLI-284
> URL: https://issues.apache.org/jira/browse/CLI-284
> Project: Commons CLI
>  Issue Type: Bug
>Reporter: Dilraj Singh
>Assignee: Bruno P. Kinoshita
>Priority: Major
> Attachments: CLI284.patch
>
>
> Builder pattern for creating an instance of class *Option* ensures that at 
> least one of the representation (short and long representation) for the 
> option is not null. It throws an *IllegalArgumentException* in-case program 
> tries to build an instance with both the representations as null. Consider 
> the following code snippet for reference.
> {code:java}
> public static void main(String[] args) {
>   Option.Builder builder = Option.builder();
>   Option op = builder.build();
> } 
> {code}
> This piece of code fails with the following exception
> {noformat}
> Exception in thread "main" java.lang.IllegalArgumentException: Either opt or 
> longOpt must be specified
> {noformat}
> But if we try to create an instance of Option by calling its constructor, It 
> allows the creation with both opt and longOpt for Option as null. Consider 
> the following code snippet for reference
> {code:java}
> public static void main(String[] args) {
>   Option op = new Option(null, null);
>   System.out.format("Short Representation: %s\n" +
>   "Long Representation: %s", op.getOpt(), 
>   op.getLongOpt());
> }
> {code}
> Output:
> {noformat}
> Short Representation: null
> Long Representation: null
> {noformat}
> Calling a method on an instance with both opt and longOpt as null will lead 
> to undesired null pointer exception. For example, calling
> {code:java}
>  op.getId() {code}
> will throw a null pointer exception, thus violating the method contract.
> So as to fix this we need to make sure that whenever a constructor is invoked 
> it has a non-null argument value for at least one of the Option 
> representation. 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CLI-284) Inconsistency in constraints for creating an instance of class Option using builder pattern or constructor

2018-06-10 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/CLI-284?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16507369#comment-16507369
 ] 

ASF GitHub Bot commented on CLI-284:


Github user sfuhrm commented on the issue:

https://github.com/apache/commons-cli/pull/25
  
Hi @kinow 

thanks for your review! Oops, my maven build had a hickup and all tests 
were green.

There's a design problem in the code you pointed me to. Every Option 
instance may be illegal (short and long == null) until first use. OptionBuilder 
(line 356) insists on creating an Option instance even if he long parameter is 
missing. Option.Builder does something similar.
The setting happens after Option instance construction.



> Inconsistency in constraints for creating an instance of class Option using 
> builder pattern or constructor
> --
>
> Key: CLI-284
> URL: https://issues.apache.org/jira/browse/CLI-284
> Project: Commons CLI
>  Issue Type: Bug
>Reporter: Dilraj Singh
>Assignee: Bruno P. Kinoshita
>Priority: Major
> Attachments: CLI284.patch
>
>
> Builder pattern for creating an instance of class *Option* ensures that at 
> least one of the representation (short and long representation) for the 
> option is not null. It throws an *IllegalArgumentException* in-case program 
> tries to build an instance with both the representations as null. Consider 
> the following code snippet for reference.
> {code:java}
> public static void main(String[] args) {
>   Option.Builder builder = Option.builder();
>   Option op = builder.build();
> } 
> {code}
> This piece of code fails with the following exception
> {noformat}
> Exception in thread "main" java.lang.IllegalArgumentException: Either opt or 
> longOpt must be specified
> {noformat}
> But if we try to create an instance of Option by calling its constructor, It 
> allows the creation with both opt and longOpt for Option as null. Consider 
> the following code snippet for reference
> {code:java}
> public static void main(String[] args) {
>   Option op = new Option(null, null);
>   System.out.format("Short Representation: %s\n" +
>   "Long Representation: %s", op.getOpt(), 
>   op.getLongOpt());
> }
> {code}
> Output:
> {noformat}
> Short Representation: null
> Long Representation: null
> {noformat}
> Calling a method on an instance with both opt and longOpt as null will lead 
> to undesired null pointer exception. For example, calling
> {code:java}
>  op.getId() {code}
> will throw a null pointer exception, thus violating the method contract.
> So as to fix this we need to make sure that whenever a constructor is invoked 
> it has a non-null argument value for at least one of the Option 
> representation. 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CLI-284) Inconsistency in constraints for creating an instance of class Option using builder pattern or constructor

2018-06-10 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/CLI-284?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16507368#comment-16507368
 ] 

ASF GitHub Bot commented on CLI-284:


Github user sfuhrm commented on the issue:

https://github.com/apache/commons-cli/pull/25
  
Hi @kinow 

thanks for your review! Oops, my maven build had a hickup and all tests 
were green.

There's a design problem in the code you pointed me to. Every Option 
instance may be illegal (short and long == null) until first use. OptionBuilder 
(line 356) insists on creating an Option instance even if he long parameter is 
missing. Option.Builder does something similar.
The setting happens after Option instance construction.



> Inconsistency in constraints for creating an instance of class Option using 
> builder pattern or constructor
> --
>
> Key: CLI-284
> URL: https://issues.apache.org/jira/browse/CLI-284
> Project: Commons CLI
>  Issue Type: Bug
>Reporter: Dilraj Singh
>Assignee: Bruno P. Kinoshita
>Priority: Major
> Attachments: CLI284.patch
>
>
> Builder pattern for creating an instance of class *Option* ensures that at 
> least one of the representation (short and long representation) for the 
> option is not null. It throws an *IllegalArgumentException* in-case program 
> tries to build an instance with both the representations as null. Consider 
> the following code snippet for reference.
> {code:java}
> public static void main(String[] args) {
>   Option.Builder builder = Option.builder();
>   Option op = builder.build();
> } 
> {code}
> This piece of code fails with the following exception
> {noformat}
> Exception in thread "main" java.lang.IllegalArgumentException: Either opt or 
> longOpt must be specified
> {noformat}
> But if we try to create an instance of Option by calling its constructor, It 
> allows the creation with both opt and longOpt for Option as null. Consider 
> the following code snippet for reference
> {code:java}
> public static void main(String[] args) {
>   Option op = new Option(null, null);
>   System.out.format("Short Representation: %s\n" +
>   "Long Representation: %s", op.getOpt(), 
>   op.getLongOpt());
> }
> {code}
> Output:
> {noformat}
> Short Representation: null
> Long Representation: null
> {noformat}
> Calling a method on an instance with both opt and longOpt as null will lead 
> to undesired null pointer exception. For example, calling
> {code:java}
>  op.getId() {code}
> will throw a null pointer exception, thus violating the method contract.
> So as to fix this we need to make sure that whenever a constructor is invoked 
> it has a non-null argument value for at least one of the Option 
> representation. 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CLI-284) Inconsistency in constraints for creating an instance of class Option using builder pattern or constructor

2018-06-10 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/CLI-284?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16507371#comment-16507371
 ] 

ASF GitHub Bot commented on CLI-284:


Github user sfuhrm closed the pull request at:

https://github.com/apache/commons-cli/pull/25


> Inconsistency in constraints for creating an instance of class Option using 
> builder pattern or constructor
> --
>
> Key: CLI-284
> URL: https://issues.apache.org/jira/browse/CLI-284
> Project: Commons CLI
>  Issue Type: Bug
>Reporter: Dilraj Singh
>Assignee: Bruno P. Kinoshita
>Priority: Major
> Attachments: CLI284.patch
>
>
> Builder pattern for creating an instance of class *Option* ensures that at 
> least one of the representation (short and long representation) for the 
> option is not null. It throws an *IllegalArgumentException* in-case program 
> tries to build an instance with both the representations as null. Consider 
> the following code snippet for reference.
> {code:java}
> public static void main(String[] args) {
>   Option.Builder builder = Option.builder();
>   Option op = builder.build();
> } 
> {code}
> This piece of code fails with the following exception
> {noformat}
> Exception in thread "main" java.lang.IllegalArgumentException: Either opt or 
> longOpt must be specified
> {noformat}
> But if we try to create an instance of Option by calling its constructor, It 
> allows the creation with both opt and longOpt for Option as null. Consider 
> the following code snippet for reference
> {code:java}
> public static void main(String[] args) {
>   Option op = new Option(null, null);
>   System.out.format("Short Representation: %s\n" +
>   "Long Representation: %s", op.getOpt(), 
>   op.getLongOpt());
> }
> {code}
> Output:
> {noformat}
> Short Representation: null
> Long Representation: null
> {noformat}
> Calling a method on an instance with both opt and longOpt as null will lead 
> to undesired null pointer exception. For example, calling
> {code:java}
>  op.getId() {code}
> will throw a null pointer exception, thus violating the method contract.
> So as to fix this we need to make sure that whenever a constructor is invoked 
> it has a non-null argument value for at least one of the Option 
> representation. 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CLI-284) Inconsistency in constraints for creating an instance of class Option using builder pattern or constructor

2018-06-09 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/CLI-284?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16507207#comment-16507207
 ] 

ASF GitHub Bot commented on CLI-284:


Github user kinow commented on the issue:

https://github.com/apache/commons-cli/pull/25
  
Hi @sfuhrm thanks for the pull request. I had a look at the CLI-284 issue 
in JIRA, and also checked out the pull request locally to take a look in 
Eclipse.

I believe we have some tests failing due to this change. See the Travis-CI 
build log, or try running `mvn clean test` locally. I got the following in my 
environment:

```
Tests run: 410, Failures: 0, Errors: 55, Skipped: 54
```

The only other comment I have is about the duplicated code that we have now.

`Option`'s constructor checks for either `opt` or `longOpt` being present. 
But so does `Option$Builder#build()`.

What about moving the 

```java
if (opt == null && longOpt == null)
{
throw new IllegalArgumentException("Either opt or longOpt must be 
specified");
}
```

check to `OptionValidator`? Maybe that way we avoid having the same if in 
two places, and running the risk of someday changing one without changing the 
other (though tests should catch it).

Thanks!


> Inconsistency in constraints for creating an instance of class Option using 
> builder pattern or constructor
> --
>
> Key: CLI-284
> URL: https://issues.apache.org/jira/browse/CLI-284
> Project: Commons CLI
>  Issue Type: Bug
>Reporter: Dilraj Singh
>Assignee: Bruno P. Kinoshita
>Priority: Major
> Attachments: CLI284.patch
>
>
> Builder pattern for creating an instance of class *Option* ensures that at 
> least one of the representation (short and long representation) for the 
> option is not null. It throws an *IllegalArgumentException* in-case program 
> tries to build an instance with both the representations as null. Consider 
> the following code snippet for reference.
> {code:java}
> public static void main(String[] args) {
>   Option.Builder builder = Option.builder();
>   Option op = builder.build();
> } 
> {code}
> This piece of code fails with the following exception
> {noformat}
> Exception in thread "main" java.lang.IllegalArgumentException: Either opt or 
> longOpt must be specified
> {noformat}
> But if we try to create an instance of Option by calling its constructor, It 
> allows the creation with both opt and longOpt for Option as null. Consider 
> the following code snippet for reference
> {code:java}
> public static void main(String[] args) {
>   Option op = new Option(null, null);
>   System.out.format("Short Representation: %s\n" +
>   "Long Representation: %s", op.getOpt(), 
>   op.getLongOpt());
> }
> {code}
> Output:
> {noformat}
> Short Representation: null
> Long Representation: null
> {noformat}
> Calling a method on an instance with both opt and longOpt as null will lead 
> to undesired null pointer exception. For example, calling
> {code:java}
>  op.getId() {code}
> will throw a null pointer exception, thus violating the method contract.
> So as to fix this we need to make sure that whenever a constructor is invoked 
> it has a non-null argument value for at least one of the Option 
> representation. 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CLI-284) Inconsistency in constraints for creating an instance of class Option using builder pattern or constructor

2018-06-09 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/CLI-284?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16507206#comment-16507206
 ] 

ASF GitHub Bot commented on CLI-284:


Github user kinow commented on the issue:

https://github.com/apache/commons-cli/pull/25
  
Hi @sfuhrm thanks for the pull request. I had a look at the CLI-284 issue 
in JIRA, and also checked out the pull request locally to take a look in 
Eclipse.

I believe we have some tests failing due to this change. See the Travis-CI 
build log, or try running `mvn clean test` locally. I got the following in my 
environment:

```
Tests run: 410, Failures: 0, Errors: 55, Skipped: 54
```

The only other comment I have is about the duplicated code that we have now.

`Option`'s constructor checks for either `opt` or `longOpt` being present. 
But so does `Option$Builder#build()`.

What about moving the 

```java
if (opt == null && longOpt == null)
{
throw new IllegalArgumentException("Either opt or longOpt must be 
specified");
}
```

check to `OptionValidator`? Maybe that way we avoid having the same if in 
two places, and running the risk of someday changing one without changing the 
other (though tests should catch it).

Thanks!


> Inconsistency in constraints for creating an instance of class Option using 
> builder pattern or constructor
> --
>
> Key: CLI-284
> URL: https://issues.apache.org/jira/browse/CLI-284
> Project: Commons CLI
>  Issue Type: Bug
>Reporter: Dilraj Singh
>Assignee: Bruno P. Kinoshita
>Priority: Major
> Attachments: CLI284.patch
>
>
> Builder pattern for creating an instance of class *Option* ensures that at 
> least one of the representation (short and long representation) for the 
> option is not null. It throws an *IllegalArgumentException* in-case program 
> tries to build an instance with both the representations as null. Consider 
> the following code snippet for reference.
> {code:java}
> public static void main(String[] args) {
>   Option.Builder builder = Option.builder();
>   Option op = builder.build();
> } 
> {code}
> This piece of code fails with the following exception
> {noformat}
> Exception in thread "main" java.lang.IllegalArgumentException: Either opt or 
> longOpt must be specified
> {noformat}
> But if we try to create an instance of Option by calling its constructor, It 
> allows the creation with both opt and longOpt for Option as null. Consider 
> the following code snippet for reference
> {code:java}
> public static void main(String[] args) {
>   Option op = new Option(null, null);
>   System.out.format("Short Representation: %s\n" +
>   "Long Representation: %s", op.getOpt(), 
>   op.getLongOpt());
> }
> {code}
> Output:
> {noformat}
> Short Representation: null
> Long Representation: null
> {noformat}
> Calling a method on an instance with both opt and longOpt as null will lead 
> to undesired null pointer exception. For example, calling
> {code:java}
>  op.getId() {code}
> will throw a null pointer exception, thus violating the method contract.
> So as to fix this we need to make sure that whenever a constructor is invoked 
> it has a non-null argument value for at least one of the Option 
> representation. 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CLI-284) Inconsistency in constraints for creating an instance of class Option using builder pattern or constructor

2018-06-09 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/CLI-284?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16506949#comment-16506949
 ] 

ASF GitHub Bot commented on CLI-284:


GitHub user sfuhrm opened a pull request:

https://github.com/apache/commons-cli/pull/25

CLI-284: Fix inconsistent behaviour for short and long name == null

Fix for https://issues.apache.org/jira/browse/CLI-284


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/sfuhrm/commons-cli master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/commons-cli/pull/25.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #25


commit 4a2e95d01e39d89ffb2711f0b13f0020fef0d561
Author: Stephan Fuhrmann 
Date:   2018-06-09T11:24:13Z

CLI-284: Fix inconsistent behaviour for short and long name == null




> Inconsistency in constraints for creating an instance of class Option using 
> builder pattern or constructor
> --
>
> Key: CLI-284
> URL: https://issues.apache.org/jira/browse/CLI-284
> Project: Commons CLI
>  Issue Type: Bug
>Reporter: Dilraj Singh
>Priority: Major
> Attachments: CLI284.patch
>
>
> Builder pattern for creating an instance of class *Option* ensures that at 
> least one of the representation (short and long representation) for the 
> option is not null. It throws an *IllegalArgumentException* in-case program 
> tries to build an instance with both the representations as null. Consider 
> the following code snippet for reference.
> {code:java}
> public static void main(String[] args) {
>   Option.Builder builder = Option.builder();
>   Option op = builder.build();
> } 
> {code}
> This piece of code fails with the following exception
> {noformat}
> Exception in thread "main" java.lang.IllegalArgumentException: Either opt or 
> longOpt must be specified
> {noformat}
> But if we try to create an instance of Option by calling its constructor, It 
> allows the creation with both opt and longOpt for Option as null. Consider 
> the following code snippet for reference
> {code:java}
> public static void main(String[] args) {
>   Option op = new Option(null, null);
>   System.out.format("Short Representation: %s\n" +
>   "Long Representation: %s", op.getOpt(), 
>   op.getLongOpt());
> }
> {code}
> Output:
> {noformat}
> Short Representation: null
> Long Representation: null
> {noformat}
> Calling a method on an instance with both opt and longOpt as null will lead 
> to undesired null pointer exception. For example, calling
> {code:java}
>  op.getId() {code}
> will throw a null pointer exception, thus violating the method contract.
> So as to fix this we need to make sure that whenever a constructor is invoked 
> it has a non-null argument value for at least one of the Option 
> representation. 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CLI-284) Inconsistency in constraints for creating an instance of class Option using builder pattern or constructor

2018-06-09 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/CLI-284?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16506950#comment-16506950
 ] 

ASF GitHub Bot commented on CLI-284:


GitHub user sfuhrm opened a pull request:

https://github.com/apache/commons-cli/pull/25

CLI-284: Fix inconsistent behaviour for short and long name == null

Fix for https://issues.apache.org/jira/browse/CLI-284


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/sfuhrm/commons-cli master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/commons-cli/pull/25.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #25


commit 4a2e95d01e39d89ffb2711f0b13f0020fef0d561
Author: Stephan Fuhrmann 
Date:   2018-06-09T11:24:13Z

CLI-284: Fix inconsistent behaviour for short and long name == null




> Inconsistency in constraints for creating an instance of class Option using 
> builder pattern or constructor
> --
>
> Key: CLI-284
> URL: https://issues.apache.org/jira/browse/CLI-284
> Project: Commons CLI
>  Issue Type: Bug
>Reporter: Dilraj Singh
>Priority: Major
> Attachments: CLI284.patch
>
>
> Builder pattern for creating an instance of class *Option* ensures that at 
> least one of the representation (short and long representation) for the 
> option is not null. It throws an *IllegalArgumentException* in-case program 
> tries to build an instance with both the representations as null. Consider 
> the following code snippet for reference.
> {code:java}
> public static void main(String[] args) {
>   Option.Builder builder = Option.builder();
>   Option op = builder.build();
> } 
> {code}
> This piece of code fails with the following exception
> {noformat}
> Exception in thread "main" java.lang.IllegalArgumentException: Either opt or 
> longOpt must be specified
> {noformat}
> But if we try to create an instance of Option by calling its constructor, It 
> allows the creation with both opt and longOpt for Option as null. Consider 
> the following code snippet for reference
> {code:java}
> public static void main(String[] args) {
>   Option op = new Option(null, null);
>   System.out.format("Short Representation: %s\n" +
>   "Long Representation: %s", op.getOpt(), 
>   op.getLongOpt());
> }
> {code}
> Output:
> {noformat}
> Short Representation: null
> Long Representation: null
> {noformat}
> Calling a method on an instance with both opt and longOpt as null will lead 
> to undesired null pointer exception. For example, calling
> {code:java}
>  op.getId() {code}
> will throw a null pointer exception, thus violating the method contract.
> So as to fix this we need to make sure that whenever a constructor is invoked 
> it has a non-null argument value for at least one of the Option 
> representation. 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CLI-284) Inconsistency in constraints for creating an instance of class Option using builder pattern or constructor

2018-06-09 Thread Stephan Fuhrmann (JIRA)


[ 
https://issues.apache.org/jira/browse/CLI-284?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16506939#comment-16506939
 ] 

Stephan Fuhrmann commented on CLI-284:
--

I think your point is right.

I'm proposing the attached patch for the project.

 

> Inconsistency in constraints for creating an instance of class Option using 
> builder pattern or constructor
> --
>
> Key: CLI-284
> URL: https://issues.apache.org/jira/browse/CLI-284
> Project: Commons CLI
>  Issue Type: Bug
>Reporter: Dilraj Singh
>Priority: Major
> Attachments: CLI284.patch
>
>
> Builder pattern for creating an instance of class *Option* ensures that at 
> least one of the representation (short and long representation) for the 
> option is not null. It throws an *IllegalArgumentException* in-case program 
> tries to build an instance with both the representations as null. Consider 
> the following code snippet for reference.
> {code:java}
> public static void main(String[] args) {
>   Option.Builder builder = Option.builder();
>   Option op = builder.build();
> } 
> {code}
> This piece of code fails with the following exception
> {noformat}
> Exception in thread "main" java.lang.IllegalArgumentException: Either opt or 
> longOpt must be specified
> {noformat}
> But if we try to create an instance of Option by calling its constructor, It 
> allows the creation with both opt and longOpt for Option as null. Consider 
> the following code snippet for reference
> {code:java}
> public static void main(String[] args) {
>   Option op = new Option(null, null);
>   System.out.format("Short Representation: %s\n" +
>   "Long Representation: %s", op.getOpt(), 
>   op.getLongOpt());
> }
> {code}
> Output:
> {noformat}
> Short Representation: null
> Long Representation: null
> {noformat}
> Calling a method on an instance with both opt and longOpt as null will lead 
> to undesired null pointer exception. For example, calling
> {code:java}
>  op.getId() {code}
> will throw a null pointer exception, thus violating the method contract.
> So as to fix this we need to make sure that whenever a constructor is invoked 
> it has a non-null argument value for at least one of the Option 
> representation. 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)