Added support for not saving options, get by name.

[timtadh]

Motivation: I don't want to save all my options. I would rather just
get options by name. There for I added some methods:

<T> T getValue(String long_form)
<T> T getValue(String long_form, T default)
<T> Collection<T> getValues(String long_form)

They work the same way as getOptionValue(Option o) etc...

A sanity test is written but more testing should probably be done.

[ooxi]

I have some issues with your patch

• You should definitely extract the test method and put it into a JUnit Testcase
• Using Strings for identifing your options means you eigher have to change two positions in your code if you change an argument's name or using a constant. When using a constant you could instead simply assign the option to a local variable

On the other hand, the long forms currently are the way the options are identified, instead of the Option's equals/hashCode methods. Therefore I'm not generally against this approach.

[timtadh]

You should definitely extract the test method and put it into a JUnit Testcase

There should probably be more testing but I believe it is in a separate test case right now. Or do you mean in a separate file?

Using Strings for identifing your options means you eigher have to change two positions in your code if you change an argument's name or using a constant. When using a constant you could instead simply assign the option to a local variable

This is true. I guess I am more of a fan of this than of having both the option variable and the value of the option variable.

Let me know what you want more specifically on the testing side and I will do it.

[ooxi]

You should definitely extract the test method and put it into a JUnit Testcase

There should probably be more testing but I believe it is in a separate test case right now. Or do you mean in a separate file?

Yes I think a separate file in the Maven test directory should be used. That way the test will run automatically on build :-)

[timtadh]

Ok. I can do that. I will create a new test file and move the test to that
file. I may also write some other tests. This is officially on my todo
list. I will update the pull request when I am done.

On Fri, Feb 15, 2013 at 8:45 AM, ooxi [email protected] wrote:

You should definitely extract the test method and put it into a JUnit
Testcase

There should probably be more testing but I believe it is in a separate
test case right now. Or do you mean in a separate file?

Yes I think a separate file in the Maven test directoryhttps://github.com/timtadh/jargs/tree/master/src/test/java/com/sanityinc/jargsshould be used. That way the test will run automatically on build :-)

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/9#issuecomment-13607031.

[ooxi]

Nevertheless I am quite sceptical about the new API. The hole point of JArgs 2.0 (in contrast to the old 1.x branch) is easy type safety: By providing the generated Option<T> instance the compiler can do all necessary type checks automatically.

If you refer to the options by name instead you will once again have to do manual type casts. For example getValues has to be invoked as

Collection<String> myStrings = myParser.<String>getValues("my-string-argument")

to get a collection of strings, which is not much better than the old way of doing

Collection<String> myStrings = (Collection<String>)myParser.getValues("my-string-argument")

Therefore I suggest you provide the new API not in the core CmdLineParser but as a derived class.

[timtadh]

Nethertheless I am quite sceptical about the new API. The hole point of
JArgs 2.0 (in contrast to the old 1.x branch) is easy type safety: By
providing the generated Option instance the compiler can do all
necessary type checks automatically.

IMO I don't think getopt implementations should be doing typing at all.
That should be done by the library user since everything starts as strings
in the first place. The getopt implementation should provide convenience
functions for the parsing. The library user will probably end up doing some
parsing anyway for more complex arguments which have some internal
structure to them. There is further input validation which needs to be done
on file paths and the like as well.

I guess my feeling is this: there is no free lunch even with a nice type
checking library like Jargs. I prefer to get my options by name for my
application. I think many programmers would disagree with this but I would
like to have the option to do it.

Therefore I suggest you provide the new API not in the core CmdLineParserbut as a derived class.

I can do that. This may be architecturally cleaner as well. I probably
should write some documentation on how to use it as well.

[purcell]

Therefore I suggest you provide the new API not in the core CmdLineParserbut as a derived class.

If it's worth doing, it's worth doing in the same class. I don't see that subclassing CmdLineParser just to keep that code separate is the right approach.

I'm sorry that I haven't yet taken the time to review the proposed code and provide substantive feedback, but I will as time allows.

[purcell]

Okay, so I took a look. As far as I can tell, you really just want to be able to pluck out the string value for "myopt", at a low level, and you don't care about any of the checking that CmdLineParser does for allowed/disallowed options.

If that's the case, splicing on a method for "just get this damn option value" isn't the approach I'd suggest. Rather, how about extracting the code in parse() into a separate helper class which parse() can then use, and which you could use directly for your purposes?

[timtadh]

Steve,

Thanks for taking a look.

Okay, so I took a look. As far as I can tell, you really just want to be
able to pluck out the string value for "myopt", at a low level, and you
don't care about any of the checking that CmdLineParser does for
allowed/disallowed options.

Basically, but as far as I can tell the parsing and type checking all still
works. It does sometimes require a cast but this seems to be a unusual
situation. eg. the following works

    CmdLineParser parser = new CmdLineParser();
    parser.addBooleanOption('h', "help");
    parser.addBooleanOption('v', "version");

    try {
        parser.parse(args);
    } catch (CmdLineParser.OptionException e) {
        System.err.println(e.getMessage());
        usage(code_option);
    }

    boolean help = parser.getValue("help", false);
    boolean version = parser.getValue("version", false);
    args = parser.getRemainingArgs();

which is how I am currently using it. You will need the type tags but not
significantly more than you need with the Option variables.

If that's the case, splicing on a method for "just get this damn option
value" isn't the approach I'd suggest. Rather, how about extracting the
code in parse() into a separate helper class which parse() can then use,
and which you could use directly for your purposes?

I just read parse() pretty carefully it looks like it would not be clean
to do such a re-factor. The easiest way would be to return a list of
(opt-name, arg-string-value) tuples. [to speak in Pythonic terms] That list
could then be post-processed by the CmdLineParser to parse the ints to
ints and so forth and place them into the values Map<String, List<?>>. The
key would be coming with an input API such that it would be reasonably easy
to specify the options "by-hand" rather than using the CmdLineParser
class.

Maybe I will take a shot at that.

[ooxi]

I agree that providing <T> T getValue(String long_form, T default) method (but I would prefer just overloading getOptionValue instead of introducting a new name) is quite convenient, but I'm not so sure about the other two methods which require explicit type casts in user code.