@Parameter on setter methods

[Alexander Kriegisch]

In issue https://github.com/cbeust/jcommander/issues/74 Cédric wrote:

Also, note that I recently added support for @parameter on top of setter methods, so you could even skip the IStringConverter part and do all the validation/conversion work in a setter method.

I wanted to try this and created a setter which takes a String and converts it into the desired type for my value, also doing validation along the way. I annotated the setter accordingly, but am having problems. First it was an IllegalAccessException, obviously because my options class was not public, but package-visible which was okay for the old parameter handling as long as the validator or converter instances were public. After changing the class to public the next problem is an InvocationTargetException. Probably this is all trivial, but I did not find anything about annotated setters in the documentation. Can you explain this please, Cédric? And do I also need a getter (currently I have none)?

[Cédric Beust ♔]

It's still an experimental feature so I'm not surprised to hear there are problems. Do you have a test case showing these problems?

-- 

Cédric

[Alexander Kriegisch]

Okay, the InvocationTargetException comes from a ParameterException I throw during validation within the setter. So obviously this exception is wrapped into the Java Reflection exception (InvocationTargetException) and rethrown that way. Obviously it is not possible (yet) to really validate and assign a value in one go using a setter. Maybe you need to think this feaute through and stabilise it. I hope I can help testing. The idea is brilliant though. We could get rid of all the tedious converter and validator interfaces.

Again the question: Do I also need a getter? If so, should it return the real value type or a String?

Am Samstag, 7. Juli 2012 00:10:18 UTC+2 schrieb Cédric Beust ♔:

It's still an experimental feature so I'm not surprised to hear there are problems. Do you have a test case showing these problems?

[Cédric Beust ♔]

Yes, you need a getter so that JCommander can figure out the default value. I could make this optional but for now, I decided to make it symmetric with fields.

Good point about validation, I'll investigate the exception part.

-- 

Cédric

[Alexander Kriegisch]

I see, thanks. An example from you would still be more helpful than me sending you a test case based on an educated guess how it might work, because if I need a getter the next questions arise:

• If I need two methods, I will not save much code anymore, compared with the converter and validator interfaces. But that is not really the main concern, I am just mentioning it.
• If the getter must return a String for JCommander, it will be next to useless for my main application because that one really needs the correctly typed value. If the getter would return the typed value, probably JCommander would not be able to handle it though. So how do I escape the dilemma?

P.S.: By trial and error now I have a working set of

• @Parameter(...) void setMyValue(String) and
• String getMyValue()
  

with the two problems discussed here: throwing a ParameterException which has an InvocationTargetException as its cause instead of the given message (it is only at the bottom of the stack trace) and the fact that the getter must return a String for JCommander. Maybe we need something like String getMyValueString (looks irritating) or getMyValueJC() or jcGetMyValue(), "jc" or "JC" meaning JCommander of course. Ugly and even more code. Maybe without a getter returning a String there should not be any validation of defaults or assigned values in this case, assuming the semantics that a setter will validate the value in any case. This way the setter alone would be enough and the user could decise whether she wants to implement a getter or allow direct read access to the variable for the main app (in my small app it is package-local access).

Just thinking aloud, I hope it is useful in any way.

[Cédric Beust ♔]

I just fixed the InvocationTargetException in master.

-- 

Cédric

[Cédric Beust ♔]

On Fri, Jul 6, 2012 at 11:29 PM, Cédric Beust ♔ <ced...@beust.com> wrote:

I just fixed the InvocationTargetException in master.

As you already figured out,  String. It's the same rules as for fields.

-- 

Cédric

[Alexander Kriegisch]

I just fixed the InvocationTargetException in master.

Acknowledged, thanks. This part works as expected now. Do you have any ideas about the rest, especially the getter having to return a String or having to exist at all? I can overload the setter and have another one which takes a typed value, but there is only one getter and I would not like it "burnt" for the rest of the application just because of JCommander.

BTW, I really appreciate in which open and quick a way you react to user concerns. And I know that flattery won't make you any faster. I wanted to say it anyway.

[Cédric Beust ♔]

Hi Alexander,

I just made getters optional and they can now return non strings. Here is an example:

  public void getterReturningNonString() {

    class Arg {

      private Integer port;

      @Parameter(names = "--port")

      public void setPort(String port) {

        this.port = Integer.parseInt(port);

      }

      public Integer getPort() {

        return port;

      }

    }

    Arg arg = new Arg();

    new JCommander(arg, new String[] { "--port", "42" });

    Assert.assertEquals(arg.port, new Integer(42));

  }

Oh and thanks for the kind words :-)

-- 

Cédric

[Alexander Kriegisch]

I tested it with a few of my parameters, and it works as expected. Thanks again!

One thing is unclear to me, though: Can this setter thing be used on list arguments (e.g. unnamed parameters) which are by default a List<String>, but converted by me into a List<Foo> in an IStringConverter? How would I do that?

[Alexander Kriegisch]

Wait, one more thing: Now that the @Parameter annotations are on the setters, the defaults are no longer displayed in the help text even though the properties beneath the setters still have default values. This is different from before and the price I just paid for saving a significant number in LOC (lines of code).

[Cédric Beust ♔]

Like I explained before, the getters are necessary to have defaults, otherwise, JCommander can't know the default for a setter @Parameter method.

-- 

Cédric

[Cédric Beust ♔]

Actually, I take that back. I just pushed a fix that will try to find a field if no getter was found for the method, so getters become "very" optional now (but convenient to convert strings to objects).

-- 

Cédric

[Alexander Kriegisch]

Tested and confirmed: defaults are visible on help screen and actionable for setter-based parameters.

Me pushing you to new frontiers and you rising to the challenge, this is a nice symbiosis. :) I did not check your changeset, but I guess you did some more reflection magic. I thought this was possible. Thumbs up!

[Cédric Beust ♔]

Awesome, thanks for all the suggestions!

-- 

Cédric