Reusable validating class attributes

Question

I have a class which implements attributes through the @property decorator as follows

class num(object):

    def __init__(self, value):
        self.val = value

    @property
    def val(self):
        return self._val

    @val.setter
    def val(self, value):
        validation(self, (int, float), value, 0.0)

When the value of the attribute is to be set, it should get validated.

def validation(obj, types, value, default):
    """Checks if the input provided for the attribute is valid."""
    try:
        if not isinstance(value, types):
            obj._val = default
            raise TypeError
        if value < 0:
            obj._val = default
            raise ValueError
        else:
            obj._val = float(value)
    except ValueError as e:
        print("Should be a positive number. Value set at ", default)
    except TypeError:
        print("Should be a number. Value set at ", default)

This code works for this particular case, where I have just one a priori known attribute val. I would like to make the function validation more general, to be able to reuse for other attributes (for example an arbitrary index). I can see that my problem resides in the obj._val, where I'm breaking encapsulation.

I guess that one possible solution is through the use of a method decorator chained to the setter of the attribute or maybe passing an extra argument to validation to be aware of which attribute has to validate. Or perhaps is by using a completely different approach.

Thanks in advance!

Answer 1

I would say one of the cleanest/reusable approaches to achieve this in python is with descriptors. The java-style get/set methods is just too verbose for this kind of requirement.

Here's what your model would look like - in just 3 lines:

class Person(object):
    balance = Integer('_balance')
    age = PositiveInteger('_age')

A few scenarios:

person = Person()

person.balance = 0  # OK
person.balance = '0'  # OK
person.balance = 'wrong'  # TypeError

person.age = 30  # OK
person.age = '30'  # OK
person.age = -1  # ValueError
person.age = 'wrong'  # TypeError

Implementation of descriptors Integer and PositiveInteger:

class Integer(object):
    def __init__(self, name, default=None):
        self.name = name
        self.default = default

    def __get__(self, instance, owner):
        return getattr(instance, self.name, self.default)

    def clean(self, value):
        if isinstance(value, int) or str(value).isdigit():
            return int(value)
        return value

    def __set__(self, instance, value):
        if isinstance(self.clean(value), int):
            setattr(instance, self.name, value)
        else:
            raise TypeError('`{}` not a valid integer'.format(value))


class PositiveInteger(Integer):
    def clean(self, value):
        x = super(PositiveInteger, self).clean(value)
        if isinstance(x, int) and x < 0:
            raise ValueError('`{}` is not a positive integer'.format(value))
        return x

Answer 2

def val(self, value):
    self._val = validation((int, float), value, 0.0)

def validation(types, value, default):
    """Checks if the input provided for the attribute is valid."""
    if not isinstance(value, types):
        print("Should be a number. Value set at ", default)
    elif value < 0:
        print("Should be a positive number. Value set at ", default)
    else:
        return value
    return default

There is no need to raise exceptions and immediately catch them. Printing immediately is fine. However using them affected the flow of the program so there was no need for an else. Since I haven't used them not only do I need an else, I need elif for the second block.

I love reducing duplication, hence the code above being how it is. For readability and robustness it might be better to do one of the following:

if ...
    print ...
    return default
if ...
    print ...
    return default
# Optionally add an else
return value

or

if ...
    print ...
    result = default
elif ...
    print ...
    result = default
else:
    result = value
return result

or if you want to make it a bit shorter reuse the parameter:

if ...
    print ...
    value = default
elif ...
    print ...
    value = default
return value

Another concise approach (for which similar variations exist) is to handle all errors at once. As well as making the code shorter it means that the user knows about all requirements in one go and doesn't fix one problem only to encounter another.

if not isinstance(value, types) or value < 0:
    print("Should be a positive number. Value set at ", default)
    return default
return value

It's not clear why you want to convert to a float if you already know you have an int or a float, so I've left that out.

Also please note that your original function didn't really use default, the setting to 0 was hardcoded which is bad.

Answer 3

Leave your validator to only validate. Have it raise an exception if the value is not valid, and at most do the float conversion for integers. Your validator is too specific to numbers, so rename it (and use other validators for other types):

def is_positive_number(value):
    """Checks if the input provided is an actual number, returns a float."""
    if not isinstance(value, types):
        raise TypeError('Not a number')
    if value < 0:
        raise ValueError('Must be a positive number')
    return float(value)

Leave setting the attribute to the setter; it can decide on a default too:

@val.setter
def val(self, value):
    self._val = 0.0  # default, in case validation fails
    self._val = validation(value)

Now the validator is no longer coupled so tightly to a specific attribute. Other, more complicated validators can be passed more context, as needed.

Note that error handling (catching the validation exception and printing the message) should really take place outside of the property. Printing inside the validator couples your code too tightly to a specific mode of interaction; you can't now use this code with a web interface or GUI, for example.