Proper use of class constants

Question

This question specifically relates to the use of the class constants ABOVE and BELOW in the sample code below.

I have a few different classes that look like this:

class MyClass(object):
    ABOVE = 1
    BELOW = 0

    def __init__(self):
        self.my_numbers = [1,2,3,4,5]

    def find_thing_in_direction(self, direction, cutoff):
        if direction == self.ABOVE:
            return [n for n in self.my_numbers if n > cutoff]
        else:
            return [n for n in self.my_numbers if n < cutoff]


my_class = MyClass()
my_var = my_class.find_thing_in_direction(MyClass.ABOVE, 3)

If I have a handful of classes scattered across different .py files that each have their own ABOVE and BELOW, should I extract these constants to somewhere, or is it better to keep the constants within their own classes?

Is there a more Pythonic way to do this instead of using these class constants?

Answer 1

I think the answer depends on the scope of your constants.

If you're trying to keep the constants the same across your handful of classes, i.e., you have many class definitions and they all use ABOVE and BELOW, and ABOVE and BELOW always have the same value, then you should pull them out. You could put them into a base class and create subclasses from that base class:

class myBaseClass(object):
    ABOVE = 1
    BELOW = 0


class mySubclass(myBaseClass):
    """ ABOVE and BELOW will be available here """
    pass

If the scope is greater than that, say module-level then put the definitions in the module __init__.py

Greater still? Then maybe move them to a settings file.

Otherwise if the scope is where you currently have it then it's ok as-is.

The line:

my_var = my_class.find_thing_in_direction(MyClass.ABOVE, 3)

puzzles me because you're referring to the constant of the Class when the instance already has it itself. It's not wrong, just differs from my reaction to use the object's own attributes:

 my_var = my_class.find_thing_in_direction(my_class.ABOVE, 3)

If you go the way of a base class, have a look at ABCs

Answer 2

You get strong kudos for not making this a boolean parameter above where passing True gets you the above search and False gets the below search. Yet it's worth taking a step back here to ask a different question first: should you even expose constants? In order to answer this, you must determine how your direction parameter will be used in practice.

As reported by Nick, Guido gave some advice for functions or methods that expose a boolean parameter. Namely, if callers "almost always [pass] a literal True or False, [it] is a good sign that there are two functions involved rather than just one." Prefer to provide these two functions—one for each case—and make the boolean parameter part of the function's name.

Your function takes an enumeration with two values, instead of a boolean, but I believe the advice holds. It takes x.ABOVE and x.BELOW, and you're trying to determine what x should be. Instead you need to figure out what the callers of your function will look like. Will they hardcode the direction, or will they be wrappers that determine the direction variably?

# Scenario 1: hardcoded "literals"
above = my_class.find_thing_in_direction(x.ABOVE, 3)
below = my_class.find_thing_in_direction(x.BELOW, 3)

# Scenario 2: passed value
result = my_class.find_thing_in_direction(direction, value)

If your callers are almost always going to be in scenario 1, that is if they almost always know the direction they are asking for, it would be better to rewrite the interface to avoid this extra parameter. It does make the second scenario a little harder, though:

# Modified for scenario 1:
above = my_class.find_thing_above(3)
below = my_class.find_thing_below(3)

# Repercussions on scenario 2:
if direction == x.ABOVE:
    result = my_class.find_thing_above(3)
else:
    result = my_class.find_thing_below(3)

As Nick said, this is not universal advice; there are exceptions. For instance, if you have more than just ABOVE and BELOW, it may be painful to expose all the various options as their own function. Consider as well the impact on the implementation. With your example code, it's trivial this way, and is arguably cleaner than the original:

def find_thing_above(self, cutoff):
    return [n for n in self.my_numbers if n > cutoff]

def find_thing_below(self, cutoff):
    return [n for n in self.my_numbers if n < cutoff]

However if the search itself was more complex, its repetition would dominate, and you will likely want to factor all search options into a single main function internally. Internally used values don't have to be quite as easily understood as values that client code might pass. In the example given by your code, perhaps you could pass a predicate:

# assume real search pattern is more complex than a single list comprehension
def _find_thing(self, predicate):
    return [n for n in self.my_numbers if predicate(n)]

def find_thing_above(self, cutoff):
    return self._find_thing(lambda n: n > cutoff)