1
\$\begingroup\$

I would love some feedback on this. I use it to validate web-form objects before persisting them in the dangerously-lazy MongoDb. I have a class and many validators for each of my collections. Is there a better way to write or go about this?

class ValidationError(Exception):
    pass    

class User(object):

    @staticmethod
    def validate(data):

        def _id(val):
            if not isinstance(val, ObjectId):
                raise ValidationError('not ObjectId')
            else:
                return val

        map_keys = dict(
            _id=lambda x: _id(x)
        )

        def count(val):
            try:
                return int(val)
            except TypeError:
                raise ValidationError('%s not integer' % val)

        new_data = {}

        for key in data:
            try:
                new_data[key] = map_keys[key](data[key])
            except KeyError:
                raise ValidationError('Key not in User: "%s"' % key)
            except ValidationError, error:
                print key, error
                raise
        return new_data
\$\endgroup\$
  • 1
    \$\begingroup\$ You probably already know this, but def int(val): ... return int(val) is recursive. \$\endgroup\$ – Greg Hewgill May 30 '14 at 0:49
1
\$\begingroup\$

lambda is redundant here as you can use _id directly as well:

    map_keys = dict(
        _id=_id
    )

Other than that, your approach seems ok. You are reinventing the wheel though and it might be a good idea to research existing libraries.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ There are python-mongodb wrappers out there which I've used that handle a lot of this stuff, but they are all too bloated for my needs. \$\endgroup\$ – MFB Jun 5 '14 at 23:23
1
\$\begingroup\$

I would reorganize things a bit as they don't seem to fit just right as they are written now.

Firstly, the different validation logic (such as _id) should not be defined inside the validate() method, and definitely not in the User class. It should be implemented as a subclass of a more general class (for example, Validator). You can have class IDValidator(Validator), class NameValidator(Validator) and so on. The Validator class should have some interface method, such as validate(), which will be implemented by all its subclasses, holding the actual validation of data.

Also, User should inherit from a more general class, like Model. Model should stand as an interface to all model-related operations (such as save, update and validate). Moreover, each model should have a fields dict, containing a mapping between your Mongo fields and their values (data from your example - which should be tied up to a model entity). Any model manipulations will be more natural this way.

To tie things up, you will have to place a Validators list in a Model's fields definition. Then, it's in your model's validate() method where you actually call each field's validator (specifically, the validator's validate() method), based on the mapping previously defined.

This way, you will have each application logic defined separately which will come in very handy when you'll need to debug things or to extend your domain by adding more models and/or validators (you won't have duplicate code).

Summarizing:

class Validator(object):
    def validate():
        raise NotImplementedError()

class IDValidator(Validator):
    def validate(self, data):
        # Do data validation and raise exceptions accordingly
        pass

class Model(object):
    def __init__(self):
        self.fields = {}
        self.validators = {}

    def validate(self):
        for field, value in self.fields:
            self.validators[field](value)

class User(Model):
    pass

As some extra things, you could consider implementing a Field class, to leverage the Model class and to make everything easier to extend. Also, it's good practice to check for errors, so be sure that, if you follow the above example, to check prior to calling the model's field validator whether it has one defined, otherwise it will throw a KeyError.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Got some great ideas from you post, thanks very much. \$\endgroup\$ – MFB Jun 5 '14 at 23:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.