91

Consider the following class:

class Person:
    def __init__(self, name, age):
        self.name = name
        self.age = age

My coworkers tend to define it like this:

class Person:
    name = None
    age = None

def __init__(self, name, age):
    self.name = name
    self.age = age

The main reason for this is that their IDE of choice shows the properties for autocompletion.

Personally, I dislike the latter one, because it makes no sense that a class has those properties set to None.

Which one would be better practice and for what reasons?

lennon310
  • 3,242
Remco Haszing
  • 1,419
  • 2
  • 12
  • 18

5 Answers5

89

I call the latter bad practice under the "this does not do what you think it does" rule.

Your coworker's position can be rewritten as: "I am going to create a bunch of class-static quasi-global variables which are never accessed, but which do take up space in the various class's namespace tables (__dict__), just to make my IDE do something."

Community
  • 1
Weaver
  • 1,079
  • 8
  • 9
33

1. Make your code easy to understand

Code is read much more often than written. Make your code maintainer's task easier (it as well may be yourself next year).

I don't know about any hard rules, but I prefer to have any future instance state clearly declared outright. Crashing with an AttributeError is bad enough. Not seeing clearly the lifecycle of an instance attribute is worse. The amount of mental gymnastic required to restore possible call sequences that lead to the attribute being assigned can easily become non-trivial, leading to errors.

So I usually not only define everything in constructor, but also strive to keep the number of mutable attributes to a minimum.

2. Don't mix class-level and instance-level members

Anything you define right inside the class declaration belongs to the class and is shared by all instances of the class. E.g. when you define a function inside a class, it becomes a method which is the same for all instances. Same applies to data members. This is totally unlike instance attributes you usually define in __init__.

Class-level data members are most useful as constants:

class Missile(object):
  MAX_SPEED = 100  # all missiles accelerate up to this speed
  ACCELERATION = 5  # rate of acceleration per game frame

  def move(self):
    self.speed += self.ACCELERATION
    if self.speed > self.MAX_SPEED:
      self.speed = self.MAX_SPEED
    # ...
9000
  • 24,342
20

Personally I define the members in the __ init__() method. I never thought about defining them in the class part. But what I always do: I init all of the members in the __ init__ method, even those not needed in the __ init__ method.

Example:

class Person:
    def __init__(self, name, age):
        self._name = name
        self._age = age
        self._selected = None

   def setSelected(self, value):
        self._selected = value

I think it is important to define all members in one place. It makes the code more readable. Whether it is inside __ init__() or outside, is not that important. But it is important for a team to commit to more or less the same coding style.

Oh, and you may notice I ever add the prefix "_" to member variables.

this.myself
  • 301
11

This is a bad practice. You don't need those values, they clutter up the code, and they can cause errors.

Consider:

>>> class WithNone:
...   x = None
...   y = None
...   def __init__(self, x, y):
...     self.x = x
...     self.y = y
... 
>>> class InitOnly:
...   def __init__(self, x, y):
...     self.x = x
...     self.y = y
... 
>>> wn = WithNone(1,2)
>>> wn.x
1
>>> WithNone.x #Note that it returns none, no error
>>> io = InitOnly(1,2)
>>> InitOnly.x
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: class InitOnly has no attribute 'x'
Daenyth
  • 8,147
0

I will go with "a bit like docstrings, then" and declare this harmless, as long as it is always None, or a narrow range of other values, all immutable.

It reeks of atavism, and excessive attachment to statically typed languages. And it does no good as code. But it has a minor purpose that remains, in documentation.

It documents what the expected names are, so if I combine code with someone and one of us has 'username' and the other user_name, there is a clue to the humans that we have parted ways and are not using the same variables.

Forcing full initialization as a policy achieves the same thing in a more Pythonic way, but if there is actual code in the __init__, this provides a clearer place to document the variables in use.

Obviously the BIG problem here is that it tempts folks to initialize with values other than None, which can be bad:

class X:
    v = {}
x = X()
x.v[1] = 2

leaves a global trace and does not create an instance for x.

But that is more of a quirk in Python as a whole than in this practice, and we should already be paranoid about it.

Jon Jay Obermark
  • 257