Skip to content

Commit d676360

Browse files
garydgregorygarydgregory
committed
Use final
- Init collection on creation - Use a switch
1 parent 4d0f3d5 commit d676360

6 files changed

Lines changed: 42 additions & 36 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ private static boolean isValidOpt(final char c) {
7878
* @return {@code true} if {@code c} was in {@code ary}, {@code false} otherwise.
7979
*/
8080
private static boolean search(final char[] chars, final char c) {
81-
for (char a : chars) {
81+
for (final char a : chars) {
8282
if (a == c) {
8383
return true;
8484
}
@@ -119,7 +119,7 @@ static String validate(final String option) throws IllegalArgumentException {
119119
return null;
120120
}
121121

122-
char[] chars = option.toCharArray();
122+
final char[] chars = option.toCharArray();
123123

124124
if (!isValidOpt(chars[0])) {
125125
throw new IllegalArgumentException("Illegal option name '" + chars[0] + "'");

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ public Options addOptionGroup(final OptionGroup group) {
159159
* @since 1.7.0
160160
*/
161161
public Options addOptions(final Options options) {
162-
for (Option opt : options.getOptions()) {
162+
for (final Option opt : options.getOptions()) {
163163
if (hasOption(opt.getKey())) {
164164
throw new IllegalArgumentException("Duplicate key: " + opt.getKey());
165165
}

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

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public class OptionValidatorTest {
7272

7373
private static Stream<Arguments> optionParameters() {
7474

75-
List<Arguments> args = new ArrayList<>();
75+
final List<Arguments> args = new ArrayList<>();
7676

7777
args.add(Arguments.of("CamelCase", true, "Camel case error"));
7878
args.add(Arguments.of("Snake_case", true, "Snake case error"));
@@ -83,24 +83,24 @@ private static Stream<Arguments> optionParameters() {
8383
args.add(Arguments.of("UPPERCASE", true, "Upper case error"));
8484

8585
// build passing test cases
86-
for (char c : firstChars.toCharArray()) {
87-
String s = String.format("%sMoreText", c);
86+
for (final char c : firstChars.toCharArray()) {
87+
final String s = String.format("%sMoreText", c);
8888
args.add(Arguments.of(s, true, String.format("testing: First character '%s'", c)));
8989
}
9090

91-
for (char c : restChars.toCharArray()) {
92-
String s = String.format("Some%sText", c);
91+
for (final char c : restChars.toCharArray()) {
92+
final String s = String.format("Some%sText", c);
9393
args.add(Arguments.of(s, true, String.format("testing: Middle character '%s'", c)));
9494
}
9595

9696
// build failing test cases
97-
for (char c : notFirstChars.toCharArray()) {
98-
String s = String.format("%sMoreText", c);
97+
for (final char c : notFirstChars.toCharArray()) {
98+
final String s = String.format("%sMoreText", c);
9999
args.add(Arguments.of(s, false, String.format("testing: Bad first character '%s'", c)));
100100
}
101101

102-
for (char c : notRestChars.toCharArray()) {
103-
String s = String.format("Some%sText", c);
102+
for (final char c : notRestChars.toCharArray()) {
103+
final String s = String.format("Some%sText", c);
104104
args.add(Arguments.of(s, false, String.format("testing: Bad middle character '%s'", c)));
105105
}
106106

@@ -110,10 +110,10 @@ private static Stream<Arguments> optionParameters() {
110110
@BeforeAll
111111
public static void setup() {
112112
StringBuilder sb = new StringBuilder();
113-
StringBuilder sb2 = new StringBuilder();
113+
final StringBuilder sb2 = new StringBuilder();
114114
int idx;
115115

116-
for (char c : PUNCTUATION.toCharArray()) {
116+
for (final char c : PUNCTUATION.toCharArray()) {
117117
if (Character.isJavaIdentifierPart(c)) {
118118
sb.append(c);
119119
} else {
@@ -124,24 +124,24 @@ public static void setup() {
124124
notAcceptablePunctuation = sb2.toString();
125125

126126
sb = new StringBuilder();
127-
for (char c : OptionValidator.ADDITIONAL_LONG_CHARS) {
127+
for (final char c : OptionValidator.ADDITIONAL_LONG_CHARS) {
128128
sb.append(c);
129129
}
130130
additionalLongChars = sb.toString();
131131

132132
sb = new StringBuilder();
133-
for (char c : OptionValidator.ADDITIONAL_OPTION_CHARS) {
133+
for (final char c : OptionValidator.ADDITIONAL_OPTION_CHARS) {
134134
sb.append(c);
135135
}
136136
additionalOptonChars = sb.toString();
137137

138-
String javaIdentifierPart = LETTERS + DIGITS + CURRENCY + acceptablePunctuation + COMBINING_MARK
138+
final String javaIdentifierPart = LETTERS + DIGITS + CURRENCY + acceptablePunctuation + COMBINING_MARK
139139
+ NON_SPACING_MARK + IDENTIFIER_IGNORABLE;
140140

141141
firstChars = additionalOptonChars + javaIdentifierPart;
142142

143143
sb = new StringBuilder(notAcceptablePunctuation).append(additionalLongChars);
144-
for (char c : OptionValidator.ADDITIONAL_OPTION_CHARS) {
144+
for (final char c : OptionValidator.ADDITIONAL_OPTION_CHARS) {
145145
while ((idx = sb.indexOf(Character.toString(c))) > -1) {
146146
sb.deleteCharAt(idx);
147147
}
@@ -150,7 +150,7 @@ public static void setup() {
150150

151151
restChars = additionalLongChars + javaIdentifierPart;
152152
sb = new StringBuilder(notAcceptablePunctuation).append(additionalOptonChars);
153-
for (char c : OptionValidator.ADDITIONAL_LONG_CHARS) {
153+
for (final char c : OptionValidator.ADDITIONAL_LONG_CHARS) {
154154
while ((idx = sb.indexOf(Character.toString(c))) > -1) {
155155
sb.deleteCharAt(idx);
156156
}
@@ -163,10 +163,10 @@ public static void setup() {
163163
public void testExclusivity() {
164164
/* since we modify acceptable chars by add and removing ADDITIONAL* chars we must verify that they do not exist in the
165165
* base javaIdentiferPart that is used in OptionValidator to validate basic characters */
166-
for (char c : OptionValidator.ADDITIONAL_LONG_CHARS) {
166+
for (final char c : OptionValidator.ADDITIONAL_LONG_CHARS) {
167167
assertFalse(Character.isJavaIdentifierPart(c), () -> String.format("'%s' should not be in 'ADDITIONAL_LONG_CHARS", c));
168168
}
169-
for (char c : OptionValidator.ADDITIONAL_OPTION_CHARS) {
169+
for (final char c : OptionValidator.ADDITIONAL_OPTION_CHARS) {
170170
assertFalse(Character.isJavaIdentifierPart(c), () -> String.format("'%s' should not be in 'ADDITIONAL_OPTION_CHARS", c));
171171
}
172172
}

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,14 +73,13 @@ public void testAddNonConflictingOptions() {
7373
options1.addOption(Option.builder("e").build());
7474
options1.addOption(Option.builder("f").build());
7575

76-
Options underTest = new Options();
76+
final Options underTest = new Options();
7777
underTest.addOptions(options1);
7878
underTest.addOptions(options2);
7979

80-
List<OptionGroup> expected = Arrays.asList(group1, group2);
80+
final List<OptionGroup> expected = Arrays.asList(group1, group2);
8181
assertTrue(expected.size() == underTest.getOptionGroups().size() && expected.containsAll(underTest.getOptionGroups()));
82-
Set<Option> expectOpt = new HashSet<>();
83-
expectOpt.addAll(options1.getOptions());
82+
final Set<Option> expectOpt = new HashSet<>(options1.getOptions());
8483
expectOpt.addAll(options2.getOptions());
8584
assertEquals(8, expectOpt.size());
8685
assertTrue(expectOpt.size() == underTest.getOptions().size() && expectOpt.containsAll(underTest.getOptions()));

‎src/test/java/org/apache/commons/cli/bug/BugCLI312Test.java‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public class BugCLI312Test {
4040
public void testNoOptionValues() {
4141
final Option o1 = Option.builder("A").build();
4242
final Option o2 = Option.builder().option("D").longOpt("define").numberOfArgs(2).valueSeparator('=').build();
43-
Options options = new Options().addOption(o1).addOption(o2);
43+
final Options options = new Options().addOption(o1).addOption(o2);
4444

4545
final CommandLineParser parser = new DefaultParser();
4646

@@ -59,7 +59,7 @@ public void testPropertyStyleOption_withGetOptionProperties() throws ParseExcept
5959
final CommandLine cl = parser.parse(options, "-Dv -Dw=1 -D x=2 -D y -D z=3 other".split(" "));
6060
assertArrayEquals(new String[] {"v", "w", "1", "x", "2", "y", "z", "3"}, cl.getOptionValues('D'));
6161

62-
Properties properties = cl.getOptionProperties("D");
62+
final Properties properties = cl.getOptionProperties("D");
6363
assertEquals("true", properties.getProperty("v"));
6464
assertEquals("1", properties.getProperty("w"));
6565
assertEquals("2", properties.getProperty("x"));
@@ -86,18 +86,25 @@ public void testPropertyStyleOption_withGetOptions() throws ParseException {
8686
if ("D".equals(o.getOpt())) {
8787
defineOptionsFound++;
8888

89-
if (defineOptionsFound == 1) {
89+
switch (defineOptionsFound) {
90+
case 1:
9091
assertArrayEquals(new String[] {"v"}, o.getValues());
91-
} else if (defineOptionsFound == 2) {
92+
break;
93+
case 2:
9294
assertArrayEquals(new String[] {"w", "1"}, o.getValues());
93-
} else if (defineOptionsFound == 3) {
95+
break;
96+
case 3:
9497
assertArrayEquals(new String[] {"x", "2"}, o.getValues());
95-
} else if (defineOptionsFound == 4) {
98+
break;
99+
case 4:
96100
assertArrayEquals(new String[] {"y"}, o.getValues());
97-
} else if (defineOptionsFound == 5) {
101+
break;
102+
case 5:
98103
assertArrayEquals(new String[] {"z", "3"}, o.getValues());
99-
} else {
104+
break;
105+
default:
100106
fail("Didn't expect " + defineOptionsFound + " occurrences of -D");
107+
break;
101108
}
102109
}
103110
}

‎src/test/java/org/apache/commons/cli/bug/BugCLI325Test.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,10 @@ public void Cli325() throws ParseException {
3939
.desc("Multiple arg option with value separator.")
4040
.build();
4141

42-
String[] args = {"-x", "A=a", "B=b"};
42+
final String[] args = {"-x", "A=a", "B=b"};
4343

44-
CommandLine cmdLine = DefaultParser.builder().build().parse(new Options().addOption(option), args);
45-
Properties props = cmdLine.getOptionProperties(option);
44+
final CommandLine cmdLine = DefaultParser.builder().build().parse(new Options().addOption(option), args);
45+
final Properties props = cmdLine.getOptionProperties(option);
4646
assertEquals(2, props.size());
4747
assertEquals("a", props.get("A"));
4848
assertEquals("b", props.get("B"));

0 commit comments

Comments
 (0)