Skip to content

Commit 0cafe22

Browse files
garydgregorygarydgregory
committed
Refactor constants
1 parent 0ece5e4 commit 0cafe22

12 files changed

Lines changed: 65 additions & 24 deletions

‎src/changes/changes.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
<action type="fix" issue="CLI-320" dev="ggregory" due-to="Paul King, Claude Warren">Awkward behavior of Option.builder() for multiple optional args.</action>
3030
<action type="fix" issue="CLI-325" dev="ggregory" due-to="Claude Warren">Properties from multiple arguments with value separator. #237.</action>
3131
<action type="fix" issue="CLI-327" dev="ggregory" due-to="Claude Warren, Gary Gregory">Fix for expected textual date values. #244.</action>
32+
<action type="fix" dev="ggregory" due-to="Gary Gregory">Option.Builder.option("") should throw IllegalArgumentException instead of ArrayIndexOutOfBoundsException.</action>
3233
<!-- ADD -->
3334
<action type="add" issue="CLI-321" dev="ggregory" due-to="Claude Warren, Gary Gregory">Add and use a Converter interface and implementations without using BeanUtils #216.</action>
3435
<action type="add" dev="ggregory" due-to="Gary Gregory">Add Maven property project.build.outputTimestamp for build reproducibility.</action>

‎src/conf/checkstyle-suppressions.xml‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,6 @@
3838
<suppress checks="HideUtilityClassConstructor" files="PatternOptionBuilder.java" />
3939
<suppress checks="HideUtilityClassConstructor" files="TypeHandler.java" />
4040

41-
<!-- Constant used in hashCode() method -->
42-
<suppress checks="MagicNumber" files="Option.java" />
43-
4441
<!-- These are final package private classes, and we do not want to hide the
4542
constructor as this will negatively affect the code coverage -->
4643
<suppress checks="HideUtilityClassConstructor" files="OptionValidator.java" />

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ public class AlreadySelectedException extends ParseException {
4141
* @since 1.2
4242
*/
4343
public AlreadySelectedException(final OptionGroup group, final Option option) {
44-
this("The option '" + option.getKey() + "' was specified but an option from this group " + "has already been selected: '" + group.getSelected() + "'",
45-
group, option);
44+
this(String.format("The option '%s' was specified but an option from this group has already been selected: '%s'", option.getKey(), group.getSelected()),
45+
group, option);
4646
}
4747

4848
/**

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,9 @@ private static String createMessage(final String option, final Collection<String
4545

4646
final Iterator<String> it = matchingOptions.iterator();
4747
while (it.hasNext()) {
48-
buf.append("'");
48+
buf.append(Char.APOS);
4949
buf.append(it.next());
50-
buf.append("'");
50+
buf.append(Char.APOS);
5151
if (it.hasNext()) {
5252
buf.append(", ");
5353
}

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/*
2+
Licensed to the Apache Software Foundation (ASF) under one or more
3+
contributor license agreements. See the NOTICE file distributed with
4+
this work for additional information regarding copyright ownership.
5+
The ASF licenses this file to You under the Apache License, Version 2.0
6+
(the "License"); you may not use this file except in compliance with
7+
the License. You may obtain a copy of the License at
8+
9+
http://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing, software
12+
distributed under the License is distributed on an "AS IS" BASIS,
13+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
See the License for the specific language governing permissions and
15+
limitations under the License.
16+
*/
17+
18+
package org.apache.commons.cli;
19+
20+
/**
21+
* Package-private character constants.
22+
*/
23+
final class Char {
24+
25+
/** Apostrophe */
26+
static final char APOS = '\'';
27+
28+
/** Carriage return. */
29+
static final char CR = '\r';
30+
31+
/** Equal sign. */
32+
static final char EQUAL = '=';
33+
34+
/** Line feed. */
35+
static final char LF = '\n';
36+
37+
/** Space. */
38+
static final char SP = ' ';
39+
40+
private Char() {
41+
// empty
42+
}
43+
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ protected void handleConcatenatedOptions(final String token) throws ParseExcepti
360360
* @param token the command line token to handle
361361
*/
362362
private void handleLongOption(final String token) throws ParseException {
363-
if (token.indexOf('=') == -1) {
363+
if (token.indexOf(Char.EQUAL) == -1) {
364364
handleLongOptionWithoutEqual(token);
365365
} else {
366366
handleLongOptionWithEqual(token);
@@ -375,7 +375,7 @@ private void handleLongOption(final String token) throws ParseException {
375375
* @param token the command line token to handle
376376
*/
377377
private void handleLongOptionWithEqual(final String token) throws ParseException {
378-
final int pos = token.indexOf('=');
378+
final int pos = token.indexOf(Char.EQUAL);
379379
final String value = token.substring(pos + 1);
380380
final String opt = token.substring(0, pos);
381381
final List<String> matchingOpts = getMatchingLongOptions(opt);
@@ -471,7 +471,7 @@ private void handleProperties(final Properties properties) throws ParseException
471471
*/
472472
private void handleShortAndLongOption(final String token) throws ParseException {
473473
final String t = Util.stripLeadingHyphens(token);
474-
final int pos = t.indexOf('=');
474+
final int pos = t.indexOf(Char.EQUAL);
475475
if (t.length() == 1) {
476476
// -S
477477
if (options.hasShortOption(t)) {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,10 @@ protected String[] flatten(final Options options, final String[] arguments, fina
6161

6262
if (options.hasOption(opt)) {
6363
tokens.add(arg);
64-
} else if (opt.indexOf('=') != -1 && options.hasOption(opt.substring(0, opt.indexOf('=')))) {
64+
} else if (opt.indexOf(Char.EQUAL) != -1 && options.hasOption(opt.substring(0, opt.indexOf(Char.EQUAL)))) {
6565
// the format is --foo=value or -foo=value
66-
tokens.add(arg.substring(0, arg.indexOf('='))); // --foo
67-
tokens.add(arg.substring(arg.indexOf('=') + 1)); // value
66+
tokens.add(arg.substring(0, arg.indexOf(Char.EQUAL))); // --foo
67+
tokens.add(arg.substring(arg.indexOf(Char.EQUAL) + 1)); // value
6868
} else if (options.hasOption(arg.substring(0, 2))) {
6969
// the format is a special properties option (-Dproperty=value)
7070
tokens.add(arg.substring(0, 2)); // -D

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ public Builder type(final Class<?> type) {
286286
* @return this builder.
287287
*/
288288
public Builder valueSeparator() {
289-
return valueSeparator('=');
289+
return valueSeparator(Char.EQUAL);
290290
}
291291

292292
/**
@@ -963,9 +963,9 @@ String toDeprecatedString() {
963963
return "";
964964
}
965965
final StringBuilder buf = new StringBuilder().append("Option '");
966-
buf.append(option).append('\'');
966+
buf.append(option).append(Char.APOS);
967967
if (longOption != null) {
968-
buf.append('\'').append(longOption).append('\'');
968+
buf.append(Char.APOS).append(longOption).append(Char.APOS);
969969
}
970970
if (isDeprecated()) {
971971
buf.append(": ").append(deprecated);
@@ -984,10 +984,10 @@ public String toString() {
984984
buf.append("Option ");
985985
buf.append(option);
986986
if (longOption != null) {
987-
buf.append(' ').append(longOption);
987+
buf.append(Char.SP).append(longOption);
988988
}
989989
if (isDeprecated()) {
990-
buf.append(' ');
990+
buf.append(Char.SP);
991991
buf.append(deprecated.toString());
992992
}
993993
if (hasArgs()) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ public static OptionBuilder withType(final Object newType) {
321321
* @return the OptionBuilder instance
322322
*/
323323
public static OptionBuilder withValueSeparator() {
324-
OptionBuilder.valueSeparator = '=';
324+
OptionBuilder.valueSeparator = Char.EQUAL;
325325

326326
return INSTANCE;
327327
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ private static boolean search(final char[] chars, final char c) {
114114
* @throws IllegalArgumentException if the Option is not valid.
115115
*/
116116
static String validate(final String option) throws IllegalArgumentException {
117-
// if opt is NULL do not check further
117+
// if opt is null do not check further.
118118
if (option == null) {
119119
return null;
120120
}

0 commit comments

Comments
 (0)