Friday, May 1, 2020

BreadcrumbsCollector: @staticmethod considered a code smell

Python offers quite a few built-in decorators that can be used to give methods of classes certain superpowers. @property turning method into a read-only field-like attribute is a classic example. Or @classmethod – a method that receives a class as a first argument, not an instance. Fun fact, this kind of method is usually called static method in other languages (e.g. Java, C#, C++ just to name a few).

Speaking of static methods, you may occasionally encounter methods decorated with @staticmethod. For instance, it often appears in codebases that are developed using PyCharm IDE because it encourages developers to change methods into static ones:

…but what are they?

What is @staticmethod?

We know already that a static method in Python is a completely different creature than similarly named constructs in other languages. “normal” accepts self as a first argument (tied to instance), @classmethod accepts cls (class) and staticmethod… does not accept either. All can, of course, have additional arguments:

class Example:
    default_multiplier = 3

    def __init__(self, multiplier: int) -> None:
        self._multiplier = multiplier

    #  tied to instance
    def normal_method(self, argument: int) -> int:
        return self._multiplier * argument

    @classmethod  # tied to class
    def class_method(cls, argument: int) -> int:
        return argument * cls.default_multiplier

    @staticmethod  # tied... but to what?!
    def static_method(argument: int) -> int:
        return argument * 2

# Calling from an instance
# all method types can be called that way
example = Example(multiplier=4)
example.normal_method(1)  # 4
example.class_method(1)  # 3
example.static_method(1)  # 2

# Calling from the class
Example.normal_method(1)  # TypeError, need two arguments (self and argument)
Example.class_method(1)  # 3
Example.static_method(1)  # 2

Although static method has no direct access to neither an instance nor class, it is still tied to the latter but in a bit more loose way – it is living in a class namespace.

In simple words, it’s like a normal function written outside but “attached” to a certain class. The only difference is that whenever someone needs to use it, has to do it via instance or class.

From the static method point of view, it has no knowledge about encompassing class whatsoever.

So why I am claiming @staticmethods are code smell?

What is a code smell?

First, a word or two of clarification because the title might suggest static methods are bad and should be avoided. That’s not the point. If I were to argue about that, I would use a word antipattern.

A code smell is a surface indication that usually corresponds to a deeper problem

http://martinfowler.com/bliki/CodeSmell.html

For those who are not familiar with the term, such a definition may still be worrying a bit. After all, if my code 💩 smells 💩 I should do something about it, right? Not necessarily. Identifying a code smell is often more of an opportunity than danger.

But why it’s even worth writing an article? @staticmethod may indicate low cohesion of a class it belongs to.

What is cohesion?

In the shortest words, cohesion stands for how strong is the relationship between class attributes. A class consists of fields and methods. The more methods use fields, the higher the cohesion. The maximum cohesion is when all methods use all class fields (unrealistic, but that’s theory ;)) If only one or two methods use a certain subset of fields, then cohesion is respectively lower.

In general, high cohesion is desirable. It means that class is focused on what it is doing. Bear in mind cohesion remains an abstract term and there is no reason to try to deliberately maximize for it. Still, when you face a code challenge or wonder how to refactor, cohesion (or lack of such) may give you invaluable hints.

What does it have to do with Python static methods?

Since static methods are, by their very definition, not using class or instance fields. Hence, they ALWAYS lower cohesion of a class. Now that we know high cohesion is desirable, should you refactor them immediately…?

No.

(…) practicality beats purity

Zen of Python

Even though a static method is not operating on class/instance data, it is still (hopefully) there for a reason. Most probably, it’s a kind of auxiliary function that’s used by other, “normal” methods.

So if it’s not a big deal when @staticmethods exist in your code, why should you even care?

Opportunity, not a danger

In a paragraph about code smells I mentioned they are more an opportunity than a danger. Further explaining that, code smells indicate an opportunity for a refactoring. Naturally, refactoring for art’s sake is nothing but a waste (unless you treat it as an exercise to practice your craft).

However, when there is a good reason to refactor, @staticmethods are probably one of the lowest hanging fruits over there. They are literally screaming Hey! You can take me out if you need to make this class smaller!

That’s one specific example of applying cohesion-based criterion to guide your refactoring. Other, idealised situation is when you can clearly identify two separate subsets of fields and methods that operate on these subsets.

class SplitPersonalityClass:
    def __init__(self, field_a: list, field_b: int) -> None:
        self._field_a = field_a
        self._field_b = field_b
        
    def add_to_a(self, arg: int) -> None:
        if len(self._field_a) > 10:
            raise ValueError
        self._field_a.append()
        
    def get_from_a(self) -> int:
        return self._field_a.pop()
    
    def multiply(self, arg: int) -> int:
        return self._field_b * arg

In this example, SplitPersonalityClass has two fields – field_a and field_b. The former is used only by first two methods while the latter is used exclusively in the last method. This class could be split into two separate classes. Of course, if there is no other reason for them to remain one (see other types of cohesion from Further Reading section).

Summary

Although @staticmethod smells a bit, it’s often no big deal. However, when facing a need for refactoring a class, they’re a perfect candidate to move them out 😉

Further reading

The post @staticmethod considered a code smell appeared first on Breadcrumbs Collector.



from Planet Python
via read more

No comments:

Post a Comment

TestDriven.io: Working with Static and Media Files in Django

This article looks at how to work with static and media files in a Django project, locally and in production. from Planet Python via read...