[bv-dev] BVAL-192 'exclusive' flag for @DecimalMin/@DecimalMax

Emmanuel Bernard emmanuel at hibernate.org
Wed Dec 12 04:09:45 EST 2012



On 11 déc. 2012, at 14:43, Gunnar Morling <gunnar at hibernate.org> wrote:

> > @DecimalMax(value = "0.0", exclusive = true)
> 
> Can we name it "inclusive" rather than "exclusive"? Personally I prefer positive variable/method names as I think they read better, in particular when it comes to negation.

I like inclusive as well

> 
> > When dealing with decimal number such a flag can indeed simplify things, 
> > however it creates a problem when dealing with error messages.
> 
> I think selecting the right message within the validator impl is sufficient. The spec could just describe which text is to be chosen in which case. Being able to make use of more advanced formatting logic (conditionals etc.) in messages is something which we should discuss separately IMO.

Yes and no. Should we postpone this issue until the formatting logic is powerful enough? Or should we go for a solution we will want to deprecate down the road?

> 
> A possible alternative would be to make use of constraint composition. E.g. @DecimalMin could be a composed constraint like this:
> 
> @Target({ METHOD, FIELD, ANNOTATION_TYPE, CONSTRUCTOR, PARAMETER })
> @Retention(RUNTIME)
> @Documented
> @DecimalMinExclusive("")
> @DecimalMinInclusive("")
> @Constraint(validatedBy = { })
> public @interface DecimalMin {
> 
>     String message() default "";
>     Class<?>[] groups() default { };
>     Class<? extends Payload>[] payload() default { };
> 
>     @OverridesAttribute.List( {
>         @OverridesAttribute(constraint=DecimalMinInclusive.class, name="value"),
>         @OverridesAttribute(constraint=DecimalMinExclusive.class, name="value") 
>     } )
>     String value();
> 
>     @OverridesAttribute.List( {
>         @OverridesAttribute(constraint=DecimalMinInclusive.class, name="inclusive"),
>         @OverridesAttribute(constraint=DecimalMinExclusive.class, name="inclusive") 
>     } )
>     boolean inclusive() default true;
> }
> 
> Depending on the value of the "inclusive" attribute, only one of the validators for @DecimalMinExclusive and @DecimalMinInclusive would validate the given value and potentially raise an error. By decomposing the validation into two distinct constraints we could make use of plain error messages:
> 
> javax.validation.constraints.DecimalMinInclusive.message  = must be greater than or equal to {value}
> javax.validation.constraints.DecimalMinExclusive.message  = must be greater than {value}
> 
> The main issue with that approach is that DecimalMinInclusive etc. would be part of the API, although they ideally shouldn't be directly used by BV end users.

I don't like this approach very much. From a user pov, it's as annoying as the double message and pollute the API with annotations they should never use. 

> 
> > I also like the approach taken in OVal. There you can create/change message 
> > variables by overriding 'Map<String, String> createMessageVariables()'. Something 
> > like this could for example be added to ConstraintValidatorContext.
> 
> A while ago I created BVAL-339 [2] which I think goes into this direction. It suggests to add a method setValue() to ConstraintValidatorContext, allowing to set arbitrary message attributes:
> 
> context
>     .buildConstraintViolationWithTemplate( "Must be after {now}" )
>     .setValue( "now", now )
>     .addConstraintViolation();
> 
> Independent from the originally discussed issue, I think adding this would makes sense in any case. If no one objects, I can create pull requests for this.
> 

I see this as less useful than the formatting improvements reusing the EL engine. So if I had to prioritize I'd put this one after. 
Do you have use cases by the way? The example with now is not really convincing :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/beanvalidation-dev/attachments/20121212/8fa967e8/attachment.html 


More information about the beanvalidation-dev mailing list