Decorator for validation - inefficient?

[Steven D'Aprano]

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