Allow an array of Classes in `validateWith` and `validateValueWith` #342

lindenb · 2017-03-25T14:33:09Z

Here is a PR where I've changed the definition of Parameter and DynamicParameter from:

  Class<? extends IParameterValidator> validateWith() default NoValidator.class;
  Class<? extends IValueValidator> validateValueWith() default NoValueValidator.class;

to

  Class<? extends IParameterValidator>[] validateWith() default NoValidator.class;
  Class<? extends IValueValidator>[] validateValueWith() default NoValueValidator.class;

this allow to specifiy multiple validator without creating a new class:

  @Parameter(names = "-age",
		  validateWith = {PositiveInteger.class,OddIntegerParameterValidator.class},
		  validateValueWith={GreaterTha0ValueValidator.class,LowerThan100ValueValidator.class})
  public int age=29;

I've added a test and a paragraph in the documentation

cbeust · 2017-03-25T14:37:38Z

I'm guessing this is backward compatible because the annotation syntax for attributes a = b allows b to be either a single value or an array?

lindenb · 2017-03-25T14:41:30Z

@cbeust

I'm guessing this is backward compatible because the annotation syntax for attributes a = b allows b to be either a single value or an array?

I'm new to jcommander. How should I test this ? can you please give me an example ?

P.

cbeust · 2017-03-25T14:47:04Z

It's not a JCommander thing: if all the tests pass with your change, you're good. I was just initially surprised how such a change could be backward compatible and then I saw that you're only modifying the attribute type from a single value to an array.

I was part of the annotation JSR and we specifically allowed "a = b" to be either a single value or an array, and that's probably why your code is backward compatible. Will review shortly.

cbeust · 2017-03-25T14:49:20Z

src/main/java/com/beust/jcommander/ParameterDescription.java

-    if (validator != null) {
-      validateParameter(this, validator, name, value);
+    Class<? extends IParameterValidator> validators[] = wrappedParameter.validateWith();
+    if (validators != null && validators.length>0) {


Spaces: length > 0.

cbeust · 2017-03-25T14:49:34Z

src/main/java/com/beust/jcommander/ParameterDescription.java

-    if (validator != null) {
-      validateValueParameter(validator, name, value);
+    Class<? extends IValueValidator> validators[] = wrappedParameter.validateValueWith();
+    if (validators != null && validators.length>0) {


cbeust · 2017-03-25T14:50:16Z

src/test/java/com/beust/jcommander/JCommanderTest.java

+    public void multipleValidatorsFails1()
+    {
+  	  ArgsMultiValidate a = new ArgsMultiValidate();
+      JCommander jc = new JCommander(a);


Indentation.

cbeust · 2017-03-25T14:50:25Z

src/test/java/com/beust/jcommander/JCommanderTest.java

+    public void multipleValidatorsFails2()
+    {
+  	  ArgsMultiValidate a = new ArgsMultiValidate();
+      JCommander jc = new JCommander(a);


Indentation.

cbeust · 2017-03-25T14:50:43Z

src/test/java/com/beust/jcommander/args/ArgsMultiValidate.java

+
+public class ArgsMultiValidate {
+  public static class OddIntegerParameterValidator implements IParameterValidator {
+	  @Override


Multiple indentations.

cbeust · 2017-03-25T14:50:55Z

src/test/java/com/beust/jcommander/args/ArgsMultiValidate.java

+  public static class OddIntegerParameterValidator implements IParameterValidator {
+	  @Override
+	public void validate(String name, String value) throws ParameterException {
+		if(Integer.parseInt(value)%2!=1) throw new ParameterException("param "+name+"="+value+" is not odd");


lindenb · 2017-03-26T12:09:05Z

back to you @cbeust. I hope I've fixed all the formatting problems :-)

cbeust · 2017-03-26T13:45:18Z

Looks good, thanks!

lindenb added 2 commits March 25, 2017 15:27

multiple validation

c4ab980

typo

7f8a496

cbeust reviewed Mar 25, 2017

View reviewed changes

fix cbeust#342 (review)

d134dd1

cbeust merged commit 60bd04c into cbeust:master Mar 26, 2017

lindenb deleted the multiplevalidators branch March 26, 2017 16:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow an array of Classes in `validateWith` and `validateValueWith` #342

Allow an array of Classes in `validateWith` and `validateValueWith` #342

lindenb commented Mar 25, 2017 •

edited

cbeust commented Mar 25, 2017

lindenb commented Mar 25, 2017

cbeust commented Mar 25, 2017

cbeust Mar 25, 2017

cbeust Mar 25, 2017

cbeust Mar 25, 2017

cbeust Mar 25, 2017

cbeust Mar 25, 2017

cbeust Mar 25, 2017

lindenb commented Mar 26, 2017

cbeust commented Mar 26, 2017

Allow an array of Classes in validateWith and validateValueWith #342

Allow an array of Classes in validateWith and validateValueWith #342

Conversation

lindenb commented Mar 25, 2017 • edited

cbeust commented Mar 25, 2017

lindenb commented Mar 25, 2017

cbeust commented Mar 25, 2017

cbeust Mar 25, 2017

Choose a reason for hiding this comment

cbeust Mar 25, 2017

Choose a reason for hiding this comment

cbeust Mar 25, 2017

Choose a reason for hiding this comment

cbeust Mar 25, 2017

Choose a reason for hiding this comment

cbeust Mar 25, 2017

Choose a reason for hiding this comment

cbeust Mar 25, 2017

Choose a reason for hiding this comment

lindenb commented Mar 26, 2017

cbeust commented Mar 26, 2017

Allow an array of Classes in `validateWith` and `validateValueWith` #342

Allow an array of Classes in `validateWith` and `validateValueWith` #342

lindenb commented Mar 25, 2017 •

edited