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 _______________________________________________ Wikidata-tech mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/wikidata-tech
