5

I am trying to wrap some of my modules into classes and have started playing with properties.

Working on combining these two answers together: making instance attribute read-only and validating attributes.

I want to be able to create an instance of DataFolder class:

df = DataFolder(owner="me", path="/.data")

After that I want to be able to allow editing the owner attribute, but not the path attribute. I want to be able to validate the attributes at the moment of initialization (both path and owner) and afterwards (yet owner only).

class DataFolder(object):
    _path = None

    #----------------------------------------------------------------------
    def __init__(self,owner,path):
        self.path = path
        self.owner = owner

    @property
    #----------------------------------------------------------------------
    def owner(self):
        return self._owner

    @owner.setter
    #----------------------------------------------------------------------
    def owner(self,owner_value):
        if "me" not in owner_value:
            raise Exception("invalid owner")
        self._owner = owner_value

    @property
    #----------------------------------------------------------------------
    def path(self):
        return self._path

    @path.setter
    #----------------------------------------------------------------------
    def path(self,path_value):
        if self._path is not None:
            raise AttributeError("Cannot edit path of existing data folder")
        if "dat" not in path_value:
            raise Exception("invalid folder")
        self._path = path_value

Is it correct/best to use the global variable _path = None and check if self._path is not None: in @path.setter? The code works fine, but I am wondering if there is a better approach.

Community
  • 1
  • 1
Alex Tereshenkov
  • 3,340
  • 8
  • 36
  • 61
  • That seems like a reasonable way to do it - it's a *class attribute*, not a *global variable*. I would raise a `ValueError` rather than base `Exception` for an invalid argument, though. As this appears to be working code, you might be better at [codereview.se]. – jonrsharpe Nov 02 '15 at 14:51
  • Thank you! Will use the ValueError instead. It's getting confusing with the "_" in front of private variables, but I hope to get used to them :) – Alex Tereshenkov Nov 02 '15 at 14:55

1 Answers1

1

It looks fine to me, except one thing: _path = None belongs to a class.

This is just a small improvment:

class DataFolder(object):
    def __init__(self, owner, path):
        self._path = None  # now it is an instance variable.
        self.path = path
        self.owner = owner

    @property
    def owner(self):
        return self._owner

    def _validate_owner(self, owner_value):
        if "me" not in owner_value:
            raise ValueError("invalid owner")

    @owner.setter
    def owner(self, owner_value):
        self._validate_owner(owner_value)
        self._owner = owner_value

    @property
    def path(self):
        return self._path

    def _validate_path(self, path_value):
        if self._path is not None:
            raise AttributeError("Cannot edit path of existing data folder")
        if "dat" not in path_value:
            raise ValueError("invalid folder")

    @path.setter
    def path(self, path_value):
        self._validate_path(path_value)
        self._path = path_value

Using:

d = DataFolder('me', 'data')
print(d.path, d.owner)
d.path = 'new_data'

Output:

('data', 'me')
new_me
AttributeError: Cannot edit path of existing data folder
sobolevn
  • 16,714
  • 6
  • 62
  • 60
  • 1
    Could you outline how you think this is better? Why not just delegate to the getter from `__init__`, rather than duplicate the code? And why are you accessing the validation on the class, not the instance (which will interfere with inheritance later on)? – jonrsharpe Nov 02 '15 at 14:58
  • But this doesn't have the behaviour of the OP's original code, i.e. that you can set one of the attributes only once... – jonrsharpe Nov 02 '15 at 15:14