89

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,132
  • 6
  • 16
  • 33
Remco Haszing
  • 1,399
  • 2
  • 11
  • 18
  • 77
    Never let your IDE dictate what code you write? – Martijn Pieters Aug 27 '14 at 11:24
  • 17
    By the way: using a proper python IDE (e.g. PyCharm), setting the attributes in the `__init__` already provides autocompletion etc. Also, using `None` prevents the IDE to infer a better type for the attribute, so it's better to use a sensible default instead (when possible). – Bakuriu Aug 27 '14 at 17:39
  • If it is just for autocompletion you can use type hinting, and the additional docstrings will be a plus too. – dashesy Feb 12 '18 at 19:20
  • If you want to predefine the attributes your instance will use, add `__slots__` to your class. – Bachsau Apr 16 '18 at 16:06
  • 3
    "Never let your IDE dictate what code you write" is a debated issue. As of Python 3.6 there's in-line annotations and a `typing` module, which allow you to provide hints to the IDE and linter, if that sort of thing tickles your fancy... – c z Apr 30 '18 at 13:57
  • 2
    These assignments at class level have no effect on the rest of the code. They have no effect on `self`. Even if `self.name` or `self.age` were not assigned in `__init__` they would not show up in the instance `self`, they only show up in the class `Person`. – jolvi May 27 '18 at 09:15
  • Actually self.name gets copied from the class variable, even if not explicitly declared, which is a little confusing. The class variable gets updated by calling . versus self.. Example: `class test(): x=1 y=7 def __init__(self,x): self.x=x self.y=x*2 def instx(self): return self.x @classmethod def classx(cls): return test.x def gety(self): print (self.y) print (test.y) return self.y a=test(2) print(a.instx()) print(a.classx()) a.gety() ` Output: `2 1 4 7 ` – MattW Apr 10 '19 at 22:04

5 Answers5

87

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,059
  • 8
  • 9
  • 6
    A bit like docstrings then ;-) Of course, Python has a handy mode to strip those, `-OO`, for those few who need it. – Steve Jessop Aug 27 '14 at 15:24
  • 18
    The space taken is by far the least important reason this convention stinks. It's a dozen bytes per name (per process). I feel like having wasted my time just *reading* the part of your answer pertaining to it, that's how unimportant it the space cost. –  Aug 27 '14 at 16:20
  • 9
    @delnan I agree about the memory size of the entries being meaningless, i was more thinkin gof the logical/mental space taken up, making introspective debugging and such require more reading and sorting, i would guess. I~LL – Weaver Aug 27 '14 at 18:46
  • 3
    Actually, if you were this paranoid about space, you would notice that this saves space, and wastes time. The uninitialized members fall through to use the class value, and therefore there aren't a bunch of copies of the variable name mapped to None (one for each instance). So the cost is a few bytes at the class and the savings is a few bytes per instance. But each failing variable name search costs a tiny bit of time. – Jon Jay Obermark Aug 27 '14 at 20:48
  • 3
    If you were worried about 8 bytes you wouldn't be using Python. – c z Apr 30 '18 at 13:58
  • So if the code at class level is removed, is it a good practice to initialize instance variables to `None` in `__init__`? PyCharm always complaint when a variable is first seen in another function. – mins Dec 22 '18 at 08:39
  • @JonJayObermark *The uninitialized members fall through to use the class value (…)* This is not the case here as both attributes are initialised in `__init__()`. – Piotr Dobrogost Mar 04 '20 at 09:48
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,162
  • 4
  • 51
  • 79
  • 2
    Yeah, but mixing class-level and instance-level members is pretty much what *def* does. It creates functions that we think of as attributes of objects, but are really members of the class. The same thing goes for *property* and its ilk. Giving the illusion that work belongs to objects when it is really mediated by the class is a time-honored way of not going nuts. How can it be all that bad, if it is OK for Python itself. – Jon Jay Obermark Aug 27 '14 at 18:54
  • 1
    Well, method resolution order in Python (also applicable to data members) is not very simple. What is not found on instance level, will be searched for on class level, then among base classes, etc. You can indeed shadow a class-level member (data or method) by assigning a same-named instance-level member. But instance-level methods are _bound_ to instance's `self` and don't need `self` to be passed, while class-level methods are _unbound_ and are plain functions as seen at `def` time, and accept an instance as first argument. So these are different things. – 9000 Aug 28 '14 at 00:05
  • I think an `AttributeError` is a a good signal than there is a bug. Otherwise, you would swallow the None and get meaningless results. Specially important in the case at hand, where the attributes are defined in the `__init__`, so a missing attribute (but existing at a class level) could only be caused by buggy inheritance. – Davidmh Aug 28 '14 at 00:11
  • @Davidmh: A detected error is always better than an undetected error, indeed! I'd say that if you have to make an attribute `None` and this value _does not make sense_ at the moment of the instance construction, you have a problem in your architecture and have to rethink the life cycle of the attribute's value or its initial value. Note that by defining your attributes early you can detect such problem even before you wrote the rest of the class, let alone running the code. – 9000 Aug 28 '14 at 02:54
  • Fun! missiles! anyway, i'm pretty sure it's OK to make class level vars and mix them... as long as the class level contains defaults, etc. – Erik Aronesty Sep 03 '19 at 18:31
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
  • 1
  • 4
  • 14
    You should set all values in the constructor. If the value was set in the class itself, it would be shared between instances, which is fine for None, but not for most values. So don't change anything about it. ;) – Remco Haszing Aug 27 '14 at 12:34
  • 4
    Default values for parameters, and the ability to take arbitrary positional and keyword arguments makes it much less painful than one would expect. (And it is actually possible to delegate construction to other methods or even stand-alone functions, so if you *really* need multiple dispatch you can do that too). – Sean Vieira Aug 27 '14 at 14:17
  • 6
    @Benedict Python has no access control. The leading underscore is the accepted convention for implementation details. See [PEP 8](http://legacy.python.org/dev/peps/pep-0008/). – Doval Aug 27 '14 at 16:07
  • 3
    @Doval There is an extraordinarily good reason to prefix attribute names with `_`: To indicate that it is private! (Why are so many people in this thread confusing or half-confusing Python with other languages?) –  Aug 27 '14 at 16:18
  • 1
    Right, so if 'ever' means 'always' in the answer, then stop! Do it only when you wish to emphasize limited access. – Jon Jay Obermark Aug 27 '14 at 20:45
  • @JonJayObermark Yes, ever means always here. As the designer and writer of a class I want to be make sure I can refactor the internal structure of a class without the risk someone is using private members. And yes, it's PEP 8. – this.myself Aug 28 '14 at 08:28
  • 1
    Ah, so you are one of those properties-or-functions-for-all-exposed-interaction people... Sorry to misunderstand. That style always seems excessive to me, but it has a following. – Jon Jay Obermark Aug 28 '14 at 14:31
  • @JonJayObermark Exactly, I am one of these people :-) As C# is my preferred programming language, it is nothing but normal to do it in Python like this. – this.myself Aug 29 '14 at 10:14
  • @doval there is some real protection when there are two underscores. always use two when you mean "private". use one when you mean "protected" – Erik Aronesty Sep 03 '19 at 18:33
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,077
  • 3
  • 31
  • 46
  • I would be hard-pressed to call that 'causing an error'. It is ambiguous what you mean by requesting 'x' here, but you might very well want the most likely initial value. – Jon Jay Obermark Aug 27 '14 at 18:51
  • I should have elaborated that it can cause an error if used incorrectly. – Daenyth Aug 27 '14 at 19:41
  • The higher risk is still initializing to an object. None really is pretty safe. The only error it is going to cause is to swallow really lame-brained exceptions. – Jon Jay Obermark Aug 27 '14 at 20:39
  • 2
    Failing to cause errors is a problem if the errors would have prevented more serious issues. – Erik Aronesty Sep 03 '19 at 18:35
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
  • 247
  • 1
  • 3