I' currently thinking about how to best validate incoming Snaks / DataValues.
I have been looking at the ValueValidator interface, and it seems to be somewhat
inconvenient.
* why do we need the setOptions() method? Shouldn't the validator be fully
configured in the constructor?
* why is there no canValidate() method? This would be very handy for finding
applicable validators for a given object.
This seems a bit odd, but where it gets really problematic is the
ValueValidatorsObject base class:
* why are errors collected in a member? I would expect a validator to be
stateless and reusable.
* why do we need the Result object? Can't we just throw an exception? The "value
with warnings" use case for MediaWiki's status object doesn't seem to apply here.
* There is a lot of "convenient stuff" implemented in the base class which i
expect many implementations are not going to need, e.g. the
allowdValues/forbiddenValues stuff.
* options are passed to sub-validators using an option map, changing the
configuration and thus behavior of the (sub-)validators on the fly. That seems
like asking for trouble.
I propose a lighter design:
* the ValueValidator interface has two methods, validate() and canValidate().
validate() can throw a validation exception, which contains one or more errors.
* Validators are stateless and get configured completely and finally by the
constructor.
* Validators should generally implement only a single constraint - so instead of
a String validator that implements min/max length, min/max value, and a regex
check, each of these checks should have their own validator; A
CompositeValidator could be used to bundle multiple validations, if needed.
* ValueValidatorFactory gets a getApplicableValidators() method that returns all
validators that can be used to validate a given object.
As far as I can see, the Validator interface isn't yet widely used, so it would
be easy enough to change these things.
What do you think?
-- daniel