Decorator for validation - inefficient?

[Bryan]

Steven D'Aprano wrote:
> On Fri, 31 Oct 2008 11:26:19 -0700, Bryan wrote:
>
> > I want my business objects to be able to do this:
> [snip code]
>
> The code you show is quite long and complex and frankly I don't have the
> time to study it in detail at the moment, but I can make a couple of
> quick comments. I see by your subject line that you're (probably)
> complaining about it being inefficient.
>
> I notice that in the validation code, you do this:
>
>
> >     def _get_invalid(self):
> >     """Collect all validation results from registered validators"""
> >         result = []
> >         for attrName in dir(self):
> >             # Prevent recursive calls
> >             if attrName == 'get_invalid' or attrName == 'invalid':
> >                 continue
> >             attr =  eval('self.' + attrName)  # Get attribute
>
> That's slow and wasteful. The right way to get an attribute is with
> getattr:
>
> attr = getattr(self, attname)
>
> I haven't tested this specifically, but in the past my timing tests
> suggest that x.attr or getattr() will run about ten times faster than
> eval('x.attr').
>
>
> >             if str(type(attr)) == "<type 'instancemethod'>":
>
> This is probably also slow and wasteful. Why convert the instance into a
> string and then do a string comparison?
>
> At the top level of your module, do this once:
>
> import new
>
> and then in the validation method do this:
>
> if type(attr) is new.instancemethod:
>
> or better still:
>
> if isinstance(attr, new.instancemethod):
>
>
>
> >                     valerr = attr()  # Get result of validation
> >                     # Validation result can be a single string, list
> >                     # of strings, or None.
> >                     # If the validation fails, it will be a string or
> >                     # list of strings
> >                     # which describe what the validation errors are.
> >                     # If the validation succeeds, None is returned.
> >                     if type(valerr) == type([]):
> >                         for err in valerr:
> >                             result.append(err)
> >                     else:
> >                         if valerr != None: result.append(valerr)
>
>
> This whole approach is unpythonic. That doesn't mean it's wrong, but it
> does suggest you should rethink it. I recommend that the validation test
> should simply raise an exception if the validation fails, and let the
> calling code deal with it.
>
>
> Hope this helps, and I may have another look at your code later today.
>
>
> --
> Steven

Thanks steven, this is the type of info I was looking for. The
instancemethod string comparison and eval() were making me cringe.

Upon review of my code I decided that I was using decorators to solve
this problem mostly because I just learned about them and I wanted to
use this cool new tool.
Instead of marking functions as being validators and then having to
inefficiently loop over all of an object's attrs to fing them all, I
am going to simply have a get_validators() function on my model
classes that my base class can call to get all the validators. Any
validators for a class would be returned in this function.

The list of validation error descriptions is returned instead of
raising exceptions so clients can show the errors to the user for
fixing. Raising exceptions seems like an uneeded burden for the
client, as there is nothing exceptional about bad user input. Instead,
I want to raise an exception if a client actually tries to save an
invalid entity back to the database. I will have to think on your
suggestion a bit more before I am sure however.

Bryan