Code-Smells and Anti-Patterns#

Code smells are coding patterns that indicate that something is wrong with the design of a programme. For example, the overuse of isinstance checks against concrete classes is a code smell, as it makes the programme more difficult to extend to deal with new types in the future.

Recognising code smells#

One way to better recognise code smells is to describe the characteristics of code. Make a note of the things you recognise; add any patterns you see, like or don’t understand. The following questions may prompt you to think further:

  • Are there methods that have the same form?

  • Are there methods that have an argument with the same name?

  • Do arguments with the same name always mean the same thing?

  • If you want to add a private method to a class, where would it go?

  • If you were to split the class into two parts, where would the dividing line be?

  • Do the tests in the conditions have anything in common?

  • How many branches do the conditions have?

  • Do the methods contain any code other than the condition?

  • Does each method depend more on the argument passed or on the class as a whole?

SOLID principles#

SOLID is an acronym for:

S – Single responsibility principle

The methods of a class should be orientated towards a single purpose.

O – Open-closed principle

Objects should be open for extensions but closed for changes.

L – Liskov’s principle of substitution

Subclasses should be substitutable by their superclasses.

I – Interface segregation principle

Objects should not depend on methods that they do not use.

D – Dependency inversion principle

Abstractions should not depend on details.

Open-closed principle#

The decision as to whether refactoring should be carried out should depend on whether your code is already open to new requirements without having to change existing code. Refactorings should not be mixed with the addition of new functions, but both processes should be separated from each other. If you are confronted with a new requirement, first reorganise the existing code so that it is open for the new function and only add the new code once this has been completed.

Refactoring is the process of changing a software system in such a way that it does not alter the external behavior of the code yet improves its internal structure.

Martin Fowler: Refactoring

Note

Safe refactoring relies on tests. If you really refactor the code without changing the behaviour, the existing tests should continue to succeed at every step. The tests are a safety net that justifies confidence in the new arrangement of the code. If they fail,

  • you have inadvertently broken the code,

  • or the existing tests are flawed.

Single responsibility principle#

The single responsibility principle states that each class should only fulfil one task:

There should never be more than one reason for a class to change.

Robert C. Martin: SRP: The Single Responsibility Principle

Liskov’s principle of substitution#

The Liskov substitution principle states that subclasses must be substitutable by their superclasses. The Liskov substitution principle also applies to Duck typing: every object that claims to be a duck must fully implement the duck’s API. Duck types should be interchangeable. Applying logic across different data types of objects is called polymorphism.

Interface segregation principle#

the interface segregation principle applies the Single responsibility principle to interfaces in order to isolate a specific behaviour. If a change to a part of your code is required, extracting an object that plays a role opens up the possibility of supporting the new behaviour without having to change the existing code. This is preferable to coded concretisations.

In this context, Demeter’s law is also interesting, which states that objects should only communicate with objects in their immediate environment. This effectively restricts the list of other objects to which an object can send a message and reduces the coupling between objects: an object can only talk to its neighbours, but not to the neighbours of its neighbours; objects can only send messages to those directly involved.

Dependency inversion principle#

The Dependency inversion principle can be defined as

Abstractions should not depend upon details. Details should depend upon abstractions.

Robert C. Martin: The Dependency Inversion Principle

Typical code smells in Python#

See also

Functions that should be objects#

In addition to object-oriented programming, Python also supports procedural programming using functions and inheritable classes. Both paradigms should, however, be applied to the appropriate problems.

Typical symptoms of functional code that should be converted to classes are

  • similar arguments across functions

  • high number of distinct Halstead operands

  • mix of mutable and immutable functions

For example, three functions with ambiguous usage can be reorganised so, that load_image() is replaced by .__init__(), crop() becomes a class method, and get_thumbnail() a property:

class Image(object):
    thumbnail_resolution = 128

    def __init__(self, path):
        ...

    def crop(self, width, height):
        ...

    @property
    def thumbnail(self):
        ...
        return thumb

Objects that should be functions#

Sometimes, however, object-oriented code should also be better broken down into functions, for example if a class contains only one other method apart from .__init__() or only static methods.

Note

You do not have to search for such classes manually, but there is a pylint rule for it:

$ pipenv run pylint --disable=all --enable=R0903 requests
************* Module requests.auth
requests/auth.py:72:0: R0903: Too few public methods (1/2) (too-few-public-methods)
requests/auth.py:100:0: R0903: Too few public methods (1/2) (too-few-public-methods)
************* Module requests.models
requests/models.py:60:0: R0903: Too few public methods (1/2) (too-few-public-methods)

-----------------------------------
Your code has been rated at 9.99/10

This shows us that two classes with only one public method have been defined in auth.py, in lines 72ff. and 100ff. Also in models.py there is a class with only one public method from line 60.

Nested code#

«Flat is better than nested.»

– Tim Peters, Zen of Python

Nested code makes it difficult to read and understand. You need to understand and remember the conditions as you go through the nestings. Objectively, the cyclomatic complexity increases as the number of code branches increases.

You can reduce nested methods with multiple nested if statements by replacing levels with methods that return False if necessary. Then you can use .count() to check if the number of errors is > 0.

Another possibility is to use list comprehensions. This way the code

results = []
for item in iterable:
    if item == match:
        results.append(item)

can be replaced by

results = [item for item in iterable if item == match]

Note

The itertools of the Python standard library are often also good for reducing the nesting depth by creating functions to create iterators from data structures.

Note

You can also filter with itertools, for example with filterfalse:

>>> from itertools import filterfalse
>>> from math import isnan
>>> from statistics import median
>>> data = [20.7, float('NaN'),19.2, 18.3, float('NaN'), 14.4]
>>> sorted(data)
[20.7, nan, 14.4, 18.3, 19.2, nan]
>>> median(data)
16.35
>>> sum(map(isnan, data))
2
>>> clean = list(filterfalse(isnan, data))
>>> clean
[20.7, 19.2, 18.3, 14.4]
>>> sorted(clean)
[14.4, 18.3, 19.2, 20.7]
>>> median(clean)
18.75

Query tools for complex dicts#

JMESPath, glom, asq and flupy can significantly simplify the query of dicts in Python.

Reduce code with dataclasses and attrs#

dataclasses

are intended to simplify the definition of classes that are mainly created to store values and can then be accessed via attribute search. Some examples are collections.namedtuple(), typing.NamedTuple, recipes for records and nested dicts. Data classes save you from having to write and manage these methods.

See also

attrs

is a Python package that has been around much longer than dataclasses, is more comprehensive and can also be used with older versions of Python.