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