Proper way to set object instance variables

Question

I'm writing a class to insert users into a database, and before I get too far in, I just want to make sure that my OO approach is clean:

class User(object):

    def setName(self,name):

        #Do sanity checks on name
        self._name = name

    def setPassword(self,password):

        #Check password length > 6 characters
        #Encrypt to md5
        self._password = password

    def commit(self):

        #Commit to database

>>u = User()
>>u.setName('Jason Martinez')
>>u.setPassword('linebreak')
>>u.commit()

Is this the right approach? Should I declare class variables up top? Should I use a _ in front of all the class variables to make them private?

Thanks for helping out.

Answer 1

There are no class variables in that code, only instance attributes. And use properties instead of accessors. And do create the instance attributes in the initializer, preferably from values passed in:

class User(object):
  def __init__(self, name, password='!!'):
    self.name = name
    self.password = password

  ...

Answer 2

It's generally correct, AFAIK, but you could clean it up with properties.

class User(object):

    def _setName(self, name=None):
        self._name = name

    def _getName(self):
        return self._name

    def _setPassword(self, password):
        self._password = password

    def _getPassword(self):
        return self._password

    def commit(self):
        pass

    name = property(_getName, _setName)
    password = property(_getPassword, _setPassword)

>>u = User()
>>u.name = 'Jason Martinez'
>>u.password = 'linebreak'
>>u.commit()

There's a also a convenient decorator-based syntax, the docs explain that too.

Answer 3

using a single _ does not make your attributes private: it is a convention telling that it is an internal attribute and should not under normal circumstances be accessed by external code. With your code it also means password and name are read only.

I strongly suggest using an initialiser for your class which will initialize your attributes, even if to a default value such as None : it will make things easier in you commit method where you won't have to check for the existence of the _name and _password attributes (with hasattr).

Use Pylint on your code.

Answer 4

Others already pointed that out: avoid using setters and getters and use simple attribute access if you don't need to perform additional logic when getting/setting your attribute. If you do need that additional logic then use properties.

If you have many parameters to pass to an instance initializer consider using separate object or dictionary holding all parameters:

>>> class User(object):
...     def __init__(self, params):
...         self.__dict__.update(params)
... 

>>> params = {
...     'username': 'john',
...     'password': 'linebreak',
...     }
>>> user = User(params)
>>> user.username
'john'
>>> user.password
'linebreak'

P.S. In your case you don't need to declare your attributes at the class level. Usually that's done if you want to share the same value across all class instances:

>>> class User(object):
...     type = 'superuser'
... 
>>> user = User()
>>> user2 = User()
>>> 
>>> user.type
'superuser'
>>> user2.type
'superuser'
>>> 
>>> user2.type = 'instance superuser'
>>> 
>>> user.type
'superuser'
>>> user2.type
'instance superuser'
>>> User.type
'superuser'