cool. actually i want to call it StringValidator, which is what it is...
and i think you're convincing me that the IValidator interface should be hidden and the getRequestX() methods removed.
there's no reason to give direct access to any of this with StringValidator and friends... we can just add a much more intuitive set of methods to an AbstractValidator base class for the validators like getIntValue(), getStringValue(), etc...
this way the whole thing will be much more self-documenting.
make sense?
only trouble is that anyone that is calling getRequestX() right now would break (in a good way, IMO) and direct IValidator implementations ought to be converted to anonymous StringValidator instantiations...
Martijn Dashorst wrote:
I can live with this solution. +1.
Martijn
Jonathan Locke wrote:
yeah, you should only occasionally need to do such a thing. and if it's a common enough thing, we ought to add a validator to the core that does it because that's the desired usage pattern.
i still really don't want to add this parameter. it was in there before and i stripped it out because i feel that it's an inelegant design. i believe there's a better solution.
for example, how about we satisfy your desire with an adapter instead? this pattern would solve your problem with the identical amount of code and would also give us a leverage point to add other features in the future without having to put the magic sauce up in Component (which is already getting crowded...). i definitely prefer this to adding that parameter...
public abstract class CustomValidator implements IValidator { public void validate(final FormComponent component) { onValidate(component, component.getRequestString()); }
protected void onValidate(FormComponent component, String value); }
Martijn Dashorst wrote:
And how about when I want to support multiple date formats? The default date implementation is not what I can use. So I implemented my own validator (inline) which parsed the request text using several different supported options. However it was not clear to use getRequestText().
Most of the time when you are implementing your own validator you will be needing the String representation, because the default converters don't work! That is why Johan (and I) propose to add the requestText as a parameter.
Martijn
Jonathan Locke wrote:
no no no! getRequestX() are for internal implementations of validators and doing model updates. you should very rarely need to do this, and so i still think we mainly have a doc problem.
validating a double is easily taken care of with new TextField(name, model, expression, class), which will automatically add a TypeValidator to your component that converts to/from the given type.
for example: new TextField("doubleValue", model, "doubleValue", Double.class)
now you have a typed text field that will automatically validate and convert to the right type. and to localize your error string, you would add
<form-name>.<component-name>.<validator-class>
or
MyForm.doubleValue.TypeValidator='${input}' is not a valid double!
you do point out that we don't have a NumberValidator that will handle double ranges. could you bug this? it's easy enough to implement for 1.0.
but if you were dealing with integers, you could simply:
myIntField.add(IntegerValidator.range(10, 50));
you can, of course, localize the error message for this too:
MyForm.myIntField.IntegerValidator=Value must be between ${min} and ${max}
the bug you report would be to add:
myDoubleField.add(NumberValdiator.range(20.0, 25.0));
all this needs to be explained in a form validation tutorial! my bad! ;-(
Johan Compagner wrote:
I find calling anything other then getRequestString very dangerous
Because for example the getRequestInt() just tries to doe parseInt() with the string. But you are still in the validate stage.
So the validator call to getRequestInt() can suddenly throw a illegal argument exception... (with a string "internal error....")
For example i can type:
23,55
That is in english/java: 23.55
But that will go very bad when i would parse that..
But i see now that getRequestAsDouble() isn't even there.. why is there a asInt or asBoolean?
I think we should first go throw the converter.. Then give that value to the validator.
If the converter fails then the validator doesn't have to be called because there is no use for that then anyway.
for example:
value given as string: "23,55"
Object object = convertor.covert("23,55", Double.class); validator.validate(component,object);
and that validator can do:
double d = ((Double)object).doubleValue(); if(d < 20 || d > 25) error("value out of range",value);
Currently building this type of validator, i must first have to get it as a string
then convert it! I need to know the locale then!!! and then do the range check..
johan
Jonathan Locke wrote:
not necessarily. the value doesn't have to be a string.
i feel this is quite elegant as it is and i think we should fix by documenting that you use getRequestX() to get the value to validate, which might be getRequestInt(), getRequestBoolean() or some other method. in particular, choices can be validated by id or index this way using getRequestInt()..
another thing we could do that would obviate the need to write validators of the kind you're suggesting is to supply validators that solve typical problems. if you're trying to validate that a value is not empty, use RequiredValidator. if you want it to have a specific value, use PatternValidator. we should document this too...
could you open a doc bug?
thx!
Johan Compagner wrote:
Hi,
This is the current method:
/**
* Validates the given input. The input corresponds to the input from the request for a
* component.
* @param component Component to validate
*/
public void validate(final FormComponent component);
But for a beginner it is not very clear what do to in this method For example:
component.getModelObjectAsString() sound very reasonalble..
But the model is not yet populated at this place.
You have to do component.getRequestString()
I don't like this, that is a line of code that has to be there in every validate method...
So we should already supply that through the method.
/**
* Validates the given input. The input corresponds to the input string from the request for the given component.
* @param component Component to validate
* @param value The value string that the user entered.
*/
public void validate(final FormComponent component, String value);
People don't make mistakes and it is much more clear that that is the value that should be validated..
johan
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
Wicket-develop mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/wicket-develop
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
Wicket-develop mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/wicket-develop
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
Wicket-develop mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/wicket-develop
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
Wicket-develop mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/wicket-develop
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
Wicket-develop mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/wicket-develop
------------------------------------------------------- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click _______________________________________________ Wicket-develop mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/wicket-develop
------------------------------------------------------- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click _______________________________________________ Wicket-develop mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/wicket-develop
------------------------------------------------------- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click _______________________________________________ Wicket-develop mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/wicket-develop
