garydgregory commented on code in PR #229:
URL: https://github.com/apache/commons-cli/pull/229#discussion_r1489662879


##########
src/test/java/org/apache/commons/cli/CommandLineTest.java:
##########
@@ -166,7 +166,27 @@ public void testGetParsedOptionValueWithOption() throws 
Exception {
     }
 
     @Test
-    public void testNullhOption() throws Exception {
+    public void testGetParsedOptionValueUsingDefault() throws Exception {
+        final Options options = new Options();
+        final Option optI = 
Option.builder("i").hasArg().type(Number.class).build();
+        final Option optF = Option.builder("f").hasArg().build();
+        options.addOption(optI);
+        options.addOption(optF);
+
+        final CommandLineParser parser = new DefaultParser();
+        final CommandLine cmd = parser.parse(options, new String[] {"-i", 
"123"});
+
+        assertEquals(123, ((Number) 
cmd.getParsedOptionValue(optI)).intValue());
+        assertEquals("foo", cmd.getParsedOptionValue(optF, "foo"));
+        assertEquals("foo", cmd.getParsedOptionValue(optF, ()-> "foo"));

Review Comment:
   `() -> ...`



##########
src/main/java/org/apache/commons/cli/CommandLine.java:
##########
@@ -437,15 +452,27 @@ public <T> T getParsedOptionValue(final Option option) 
throws ParseException {
      * @see PatternOptionBuilder
      * @since 1.7.0
      */
-    @SuppressWarnings("unchecked")
     public <T> T getParsedOptionValue(final Option option, final T 
defaultValue) throws ParseException {
-        if (option == null) {
-            return null;
-        }
-        final String res = getOptionValue(option);
+        return getParsedOptionValue(option, () -> defaultValue);
+    }
+
+    /**
+     * Gets a version of this {@code Option} converted to a particular type.
+     *
+     * @param option the name of the option.
+     * @param defaultValue the default value to return if opt is not set.
+     * @param <T> The return type for the method.
+     * @return the value parsed into a particular object.
+     * @throws ParseException if there are problems turning the option value 
into the desired type
+     * @see PatternOptionBuilder
+     * @since 1.7.0
+     */
+    @SuppressWarnings("unchecked")
+    public <T> T getParsedOptionValue(final Option option, final Supplier<T> 
defaultValue) throws ParseException {
+        final String res = option == null ? null : getOptionValue(option);
 
         try {
-            return res == null ? defaultValue : (T) 
option.getConverter().apply(res);
+            return res == null ? defaultValue.get() : (T) 
option.getConverter().apply(res);

Review Comment:
   Hi @Claudenw 
   You're missing a test for when `defaultValue` is null.



##########
src/main/java/org/apache/commons/cli/CommandLine.java:
##########
@@ -423,7 +438,7 @@ public <T> T getParsedOptionValue(final char opt, final T 
defaultValue) throws P
      * @since 1.5.0
      */
     public <T> T getParsedOptionValue(final Option option) throws 
ParseException {
-        return  getParsedOptionValue(option, null);
+        return  getParsedOptionValue(option, ()-> null);

Review Comment:
   Missing a space after `()`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to