Common anti-patterns in Python

Making analysis results more useful and relevant is one of our primary goals at DeepSource, and we are regularly adding more capabilities to our core analyzers.

1. Not using with to open files

When you open a file without the with statement, you need to remember closing the file via calling close() explicitly when finished with processing it. Even while explicitly closing the resource, there are chances of exceptions before the resource is actually released. This can cause inconsistencies, or leading the file to be corrupted. Opening a file via with implements the context manager protocol that release the resource when execution is outside of the with block.

Bad:

new_file = open('some-file.txt', 'r')
# do something exciting
new_file.close()

Good:

with open('some-file.txt', 'r') as fd:
    data = fd.read()
  # do something exciting

2. Using list/dict/set comprehension unnecessarily

Built-in functions like all, any, enumerate, iter, itertools.cycle, itertools.accumulate, can work directly with a generator expression. They do not require comprehension.

In addition to them: all() and any() in Python also support short-circuiting, but this behavior is lost if comprehension is used. This affects performance.

Bad practice:

...
comma_seperated_names = ','.join([name for name in my_fav_superheroes])

Good practice:

...
comma_seperated_numbers = ','.join(name for name in my_fav_superheroes)

3. Unnecessary use of generators

It is unnecessary to use a generator expression within a call to list, dict or set since there are comprehensions for each of these types. Instead of using list/dict/set around a generator expression, they can be written as their respective comprehension.

Bad practice:

squares = dict((i,i**2) for i in range(1,10))

Good practice:

squares = {i: i**2 for i in range(1,10)}

4. Returning more than one object type in function call

Having inconsistent return types in a function makes the code confusing and complex to understand, and can lead to bugs which are hard to resolve. If a function is supposed to return a given type (e.g. integer constant, list, tuple) but can something else as well, the caller of that function will always need to check for the type of value being returned. It is recommended to return only one type of object from a function.

If there's a need to return something empty in some failing case, it is recommended to raise an exception that can be cleanly caught.

Bad practice:

def get_person_age(name):
    person = db.get_person(name)
    if person:
        return person.age  # returns an int

    # returns None if person not found

Good practice:

def get_person_age(name):
    person = db.get_person(name)
    if not person:
        raise Exception(f'No person found with name {name}')
    return person.age  # guaranteed to return int every time

5. Not using get() to return default values from a dictionary

This anti-pattern affects the readability of code. Often we see code to create a variable, assign a default value to it, and then checking the dictionary for a certain key. If the key exists, then the value of the key is assigned into the value for the variable. Although there is nothing wrong in doing this, it is verbose and inefficient as it queries the dictionary twice, while it can be easily done using the get() method for the dictionary.

Bad practice:

currency_map = {'usd': 'US Dollar'}

if 'inr' in currency_map:
  indian_currency_name = currency_map['inr']
else:
  indian_currency_name = 'undefined'

Good practice:

currency_map = {'usd': 'US Dollar'}
indian_currency_name = currency_map.get('inr', 'undefined')

6. Not using items() to iterate over a dictionary

The items method on a dictionary returns an iterable with key-value tuples which can be unpacked in a for loop. This approach is idiomatic, and hence recommended.

Bad practice:

for code in country_map:
    name = country_map[code]
    # do something with name

Good practice:

for code, name in country_map.items():
    # do something with name
    pass

7. Not using literal syntax to initialize empty list/dict/tuple

It is relatively slower to initialize an empty dictionary by calling dict() than using the empty literal, because the name dict must be looked up in the global scope in case it has been rebound. Same goes for the other two types — list() and tuple().

Bad practice:

my_evans = list()
# Add something to this empty list

Good practice:

my_evans = []
# Add something to this empty list

8. Pushing debugger in production code

Most of us have done this at least once — while debugging code, it may happen that you push your code after you found the bug but forgotten to remove the debugger. This is critical, and can affect the behavior of the code. It is highly recommended to audit the code to remove invocations of debuggers before checking in.

After latest update, the Python analyzer now detects all of these anti-patterns, which can help you avoid some common gotchas. We're actively adding more anti-patterns to our analyzers. What other anti-patterns would you like to see? Let us know @DeepSourceHQ.