Readability: Do not abuse classes as global state

You know that passing state around different functions by using global variables is bad: it results in spaghetti code, it introduces side-effects to your functions and, well, is just bad practice. Then: don’t make the same mistake when using classes.

The form this manifests in code is by having a particular class method (not necessarily the constructor!) initializing a member field and later having other unrelated methods querying the attribute “out of the blue”.

Consider the following code in which we have the definition of a class to represent a real-world object and a caller test method:

class Table(object):

    def __init__(self, database, label):
        self._database = db
        self._label = label

        self.depth = None
        self.height = None
        self.width = None

    def query_dimensions(self):
        self.depth = self._database.get(self._label, 'depth')
        self.height = self._database.get(self._label, 'height')
        self.width = self._database.get(self._label, 'width')

    def volume(self):
        self.query_dimensions()
        return self.depth * self.height * self.width

def test():
    table = Table('foo', Database.connect())
    table.query_dimensions()
    print table.depth, table.height, table.width
    print table.volume()

Yes, I’ve seen something similar in production. No, this is not good. There are many problems with this design, but let’s focus on the one that is the point of this article.

The specific problem I want to highlight is in the query_dimensions() method: such method is abusing the local attributes as input parameters and is also storing its various return values into attributes. The fact that this method is public is even worse, as illustrated by the need to call it before being able to access public attributes.

Conceptually, a function to retrieve values from a database should be receiving the database and the object identifier as parameters, and return the results of the query. We want the function to not use global state and instead look like this:

@staticmethod
def query_dimensions(database, label):
    depth = database.get(label, 'depth')
    height = database.get(label, 'height')
    width = database.get(label, 'width')
    return depth, height, width

Note that this function has no hidden dependencies: all data read or written is accessed via input parameters or returned via return values.

Now, plugging this into our previous class allows us to “fix” the volume() method to not rely on updating the local variables in a magic manner. However, having changed the function this way exposes that the various public attributes cannot now be accessed from outside. We need to rewrite the code quite heavily:

class Table(object):

    def __init__(self, database, label):
        self._database = db
        self._label = label

        self._depth = None
        self._height = None
        self._width = None

    @staticmethod
    def _query_dimensions(database, label):
        depth = database.get(label, 'depth')
        height = database.get(label, 'height')
        width = database.get(label, 'width')
        return depth, height, width

    def depth(self):
        if self._depth is None:
            self._depth, self._height, self._width = (
                self._query_dimensions(self._database, self._label)
        return self._depth

    def height(self):
        if self._height is None:
            self._depth, self._height, self._width = (
                self._query_dimensions(self._database, self._label)
        return self._height

    def width(self):
        if self._width is None:
            self._depth, self._height, self._width = (
                self._query_dimensions(self._database, self._label)
        return self._width

    def volume(self):
        return self.depth() * self.height() * self.width()

def test():
    table = Table('foo', Database.connect())
    print table.depth(), table.height(), table.width()
    print table.volume()

This new code is certainly much longer than the original code. However, this updated version is easier to follow because the points at which attributes get updated have been made explicit. Also, this offers better encapsulation by avoiding the caller to know how to recompute internal values. And this variant highlights that, maybe, we ought to have a Dimensions type to group the depth, height and width of an object.

To summarize: don’t use attributes for data that only need to be passed around methods. Attempt to make your methods independent from the instance, or at least think about them this way so that you realize what dependencies they have.