Skip to content

Commit faa6455

Browse files
netominetomi
committed
[CLI-224] Add static builder methods to Option, check if at least one of opt/longOpt has been specified, update javadoc.
git-svn-id: https://svn.apache.org/repos/asf/commons/proper/cli/trunk@1447005 13f79535-47bb-0310-9956-ffa450edef68
1 parent b06091d commit faa6455

4 files changed

Lines changed: 82 additions & 38 deletions

File tree

‎src/main/java/org/apache/commons/cli/Option.java‎

Lines changed: 53 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@
2828
* this option, and a self-documenting description of the option.
2929
* <p>
3030
* An Option is not created independently, but is created through
31-
* an instance of {@link Options}.
31+
* an instance of {@link Options}. An Option is required to have
32+
* at least a short or a long-name.
3233
* <p>
3334
* <b>Note:</b> once an {@link Option} has been added to an instance
3435
* of {@link Options}, it's required flag may not be changed anymore.
@@ -750,27 +751,53 @@ boolean requiresArg()
750751
}
751752
}
752753

754+
/**
755+
* Returns a {@link Builder} to create an {@link Option} using descriptive
756+
* methods.
757+
*
758+
* @return a new {@link Builder} instance
759+
* @since 1.3
760+
*/
761+
public static Builder builder()
762+
{
763+
return builder(null);
764+
}
765+
766+
/**
767+
* Returns a {@link Builder} to create an {@link Option} using descriptive
768+
* methods.
769+
*
770+
* @param opt short representation of the option
771+
* @return a new {@link Builder} instance
772+
* @throws IllegalArgumentException if there are any non valid Option characters in {@code opt}
773+
* @since 1.3
774+
*/
775+
public static Builder builder(final String opt)
776+
{
777+
return new Builder(opt);
778+
}
779+
753780
/**
754781
* A nested builder class to create <code>Option</code> instances
755782
* using descriptive methods.
756783
* <p>
757784
* Example usage:
758785
* <pre>
759-
* Option option = new Option.Builder("a", "Long description")
786+
* Option option = Option.builder("a")
760787
* .required(true)
761788
* .longOpt("arg-name")
762789
* .build();
763790
* </pre>
764791
*
765792
* @since 1.3
766793
*/
767-
public static class Builder
794+
public static final class Builder
768795
{
769796
/** the name of the option */
770797
private final String opt;
771798

772799
/** description of the option */
773-
private final String description;
800+
private String description;
774801

775802
/** the long representation of the option */
776803
private String longOpt;
@@ -793,29 +820,17 @@ public static class Builder
793820
/** the character that is the value separator */
794821
private char valuesep;
795822

796-
/**
797-
* Constructs a new <code>Builder</code>.
798-
*/
799-
public Builder()
800-
{
801-
this(null, null);
802-
}
803-
804823
/**
805824
* Constructs a new <code>Builder</code> with the minimum
806825
* required parameters for an <code>Option</code> instance.
807826
*
808827
* @param opt short representation of the option
809-
* @param description describes the function of the option
810-
* @throws IllegalArgumentException if there are any non valid
811-
* Option characters in <code>opt</code>.
828+
* @throws IllegalArgumentException if there are any non valid Option characters in {@code opt}
812829
*/
813-
public Builder(final String opt, final String description)
814-
throws IllegalArgumentException
830+
private Builder(final String opt) throws IllegalArgumentException
815831
{
816832
OptionValidator.validateOption(opt);
817833
this.opt = opt;
818-
this.description = description;
819834
}
820835

821836
/**
@@ -829,7 +844,19 @@ public Builder argName(final String argName)
829844
this.argName = argName;
830845
return this;
831846
}
832-
847+
848+
/**
849+
* Sets the description for this option.
850+
*
851+
* @param description the description of the option.
852+
* @return this builder, to allow method chaining
853+
*/
854+
public Builder desc(final String description)
855+
{
856+
this.description = description;
857+
return this;
858+
}
859+
833860
/**
834861
* Sets the long name of the Option.
835862
*
@@ -918,12 +945,17 @@ public Builder hasArg(final boolean hasArg)
918945
}
919946

920947
/**
921-
* Constructs an Option.
948+
* Constructs an Option with the values declared by this {@link Builder}.
922949
*
923-
* @return the new Option
950+
* @return the new {@link Option}
951+
* @throws IllegalArgumentException if neither {@code opt} or {@code longOpt} has been set
924952
*/
925953
public Option build()
926954
{
955+
if (opt == null && longOpt == null)
956+
{
957+
throw new IllegalArgumentException("Either opt or longOpt must be specified");
958+
}
927959
return new Option(this);
928960
}
929961
}

‎src/main/java/org/apache/commons/cli/OptionBuilder.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
*
2828
* @version $Id$
2929
* @since 1.0
30-
* @deprecated since 1.3, use {@link Option.Builder} instead
30+
* @deprecated since 1.3, use {@link Option.builder(String)} instead
3131
*/
3232
@Deprecated
3333
public final class OptionBuilder

‎src/main/java/org/apache/commons/cli/PatternOptionBuilder.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ public static Options parsePattern(String pattern)
160160
{
161161
if (opt != ' ')
162162
{
163-
final Option option = new Option.Builder(String.valueOf(opt), null)
163+
final Option option = Option.builder(String.valueOf(opt))
164164
.hasArg(type != null)
165165
.required(required)
166166
.type(type)
@@ -187,7 +187,7 @@ else if (ch == '!')
187187

188188
if (opt != ' ')
189189
{
190-
final Option option = new Option.Builder(String.valueOf(opt), null)
190+
final Option option = Option.builder(String.valueOf(opt))
191191
.hasArg(type != null)
192192
.required(required)
193193
.type(type)

‎src/test/java/org/apache/commons/cli/OptionTest.java‎

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -153,36 +153,48 @@ public void testBuilderMethods()
153153
{
154154
char defaultSeparator = (char) 0;
155155

156-
checkOption(new Option.Builder("a", "desc").build(),
156+
checkOption(Option.builder("a").desc("desc").build(),
157157
"a", "desc", null, Option.UNINITIALIZED, null, false, false, defaultSeparator, String.class);
158-
checkOption(new Option.Builder("a", "desc").build(),
158+
checkOption(Option.builder("a").desc("desc").build(),
159159
"a", "desc", null, Option.UNINITIALIZED, null, false, false, defaultSeparator, String.class);
160-
checkOption(new Option.Builder("a", "desc").longOpt("aaa").build(),
160+
checkOption(Option.builder("a").desc("desc").longOpt("aaa").build(),
161161
"a", "desc", "aaa", Option.UNINITIALIZED, null, false, false, defaultSeparator, String.class);
162-
checkOption(new Option.Builder("a", "desc").hasArg(true).build(),
162+
checkOption(Option.builder("a").desc("desc").hasArg(true).build(),
163163
"a", "desc", null, 1, null, false, false, defaultSeparator, String.class);
164-
checkOption(new Option.Builder("a", "desc").hasArg(false).build(),
164+
checkOption(Option.builder("a").desc("desc").hasArg(false).build(),
165165
"a", "desc", null, Option.UNINITIALIZED, null, false, false, defaultSeparator, String.class);
166-
checkOption(new Option.Builder("a", "desc").hasArg(true).build(),
166+
checkOption(Option.builder("a").desc("desc").hasArg(true).build(),
167167
"a", "desc", null, 1, null, false, false, defaultSeparator, String.class);
168-
checkOption(new Option.Builder("a", "desc").numberOfArgs(3).build(),
168+
checkOption(Option.builder("a").desc("desc").numberOfArgs(3).build(),
169169
"a", "desc", null, 3, null, false, false, defaultSeparator, String.class);
170-
checkOption(new Option.Builder("a", "desc").required(true).build(),
170+
checkOption(Option.builder("a").desc("desc").required(true).build(),
171171
"a", "desc", null, Option.UNINITIALIZED, null, true, false, defaultSeparator, String.class);
172-
checkOption(new Option.Builder("a", "desc").required(false).build(),
172+
checkOption(Option.builder("a").desc("desc").required(false).build(),
173173
"a", "desc", null, Option.UNINITIALIZED, null, false, false, defaultSeparator, String.class);
174174

175-
checkOption(new Option.Builder("a", "desc").argName("arg1").build(),
175+
checkOption(Option.builder("a").desc("desc").argName("arg1").build(),
176176
"a", "desc", null, Option.UNINITIALIZED, "arg1", false, false, defaultSeparator, String.class);
177-
checkOption(new Option.Builder("a", "desc").optionalArg(false).build(),
177+
checkOption(Option.builder("a").desc("desc").optionalArg(false).build(),
178178
"a", "desc", null, Option.UNINITIALIZED, null, false, false, defaultSeparator, String.class);
179-
checkOption(new Option.Builder("a", "desc").optionalArg(true).build(),
179+
checkOption(Option.builder("a").desc("desc").optionalArg(true).build(),
180180
"a", "desc", null, Option.UNINITIALIZED, null, false, true, defaultSeparator, String.class);
181-
checkOption(new Option.Builder("a", "desc").valueSeparator(':').build(),
181+
checkOption(Option.builder("a").desc("desc").valueSeparator(':').build(),
182182
"a", "desc", null, Option.UNINITIALIZED, null, false, false, ':', String.class);
183-
checkOption(new Option.Builder("a", "desc").type(Integer.class).build(),
183+
checkOption(Option.builder("a").desc("desc").type(Integer.class).build(),
184184
"a", "desc", null, Option.UNINITIALIZED, null, false, false, defaultSeparator, Integer.class);
185185
}
186+
187+
@Test(expected=IllegalArgumentException.class)
188+
public void testBuilderInsufficientParams1()
189+
{
190+
Option.builder().desc("desc").build();
191+
}
192+
193+
@Test(expected=IllegalArgumentException.class)
194+
public void testBuilderInsufficientParams2()
195+
{
196+
Option.builder(null).desc("desc").build();
197+
}
186198

187199
private static void checkOption(Option option, String opt, String description, String longOpt, int numArgs,
188200
String argName, boolean required, boolean optionalArg,

0 commit comments

Comments
 (0)