Programming pet peeves

01 Jul 2017

Here’s a list of pet peeves I happen to have. The examples are in Python, but only the first one is Python-specific. I might make a series of these posts.

Using mutable data structures as default arguments in Python

Example:

def some_function(a, b, c={}):
    c[a] = b
    return c

Why it’s bad:

>>> some_function('key1', 'value1')
{'key1': 'value1'}

>>> some_function('key2', 'value2')
{'key1': 'value1', 'key2': 'value2'}

As you can see, it’s mutating the same Python dictionary specified as the default keyword argument!

Solution:

Use None instead.

def better_function(a, b, c=None):
    c = c or {}
    c[a] = b
    return c

better_function('key1', 'value1')  # {'key1': 'value1'}
better_function('key2', 'value2')  # {'key2': 'value2'}

Much better.

Direct references to third party libraries everywhere in code

Example:

Say you installed a 3rd party module called somelibrary that you’re using in your Django project.

This is what usually happens:

import somelibrary
from django.shortcuts import render
from django.http import Http404


def view_one(request):
    r = somelibrary.call_api(action='test')
    if r.is_success():
        return render(request, 'success.html', {'token': r.token})
    raise Http404

def view_two(request):
    r = somelibrary.call_api(action='some_other_action')
    if r.is_success():
        return render(request, 'success.html', {'token': r.token})
    raise Http404

Why it’s bad:

Imagine you’re using this same pattern in a hundred different places.

Now imagine you upgrade somelibrary, and the API changes. call_api() is no longer a function in the module, and instead you have to call specific action functions:

def view_one(request):
    r = somelibrary.test()  # API changed here
    if r.no_errors():  # API changed here too
        return render(request, 'success.html', {'token': r.token})
    raise Http404

def view_two(request):
    r = somelibrary.some_other_action()  # API changed here
    if r.no_errors():  # API changed here too
        return render(request, 'success.html', {'token': r.token})
    raise Http404

You’ll have to search and replace every instance where you used the old API. Sometimes it’s not as straightforward as a search and replace, e.g when you rely on a specific sequence of events. Of course, it’s not that bad when you don’t use it everywhere, but things can quickly grow out of control.

Solution:

If you use the external library extensively, write a simple wrapper that delegates to it.

For example:

# wrapper.py
import somelibrary


class SomeLibraryResponse:
    def __init__(self, response_from_somelibrary):
        self.response = response_from_somelibrary

    def is_success(self):
        return self.response.is_success()


class SomeLibraryWrapper:
    def test(self):
        return SomeLibraryResponse(somelibrary.call_api(action='test'))

    def some_other_action(self):
        return SomeLibraryResponse(somelibrary.call_api(action='some_other_action'))

So you will use these wrappers instead of directly calling somelibrary.

What’s the benefit of this? If you are using the wrapper instead of directly referencing somelibrary, any API changes to somelibrary will now only require changing the wrapper. So in our example, we only have to change our wrapper to this, instead of hundreds of places:

# wrapper.py
import somelibrary


class SomeLibraryResponse:
    def __init__(self, response_from_somelibrary):
        self.response = response_from_somelibrary

    def is_success(self):
        # API Changed here. Notice our method signature of our wrapper
        # remains the same
        return self.response.no_errors()


class SomeLibraryWrapper:
    def test(self):
        # API Changed here. Notice our method signature of our wrapper
        # remains the same
        return SomeLibraryResponse(somelibrary.test())

    def some_other_action(self):
        # API Changed here. Notice our method signature of our wrapper
        # remains the same
        return SomeLibraryResponse(somelibrary.some_other_action())

Classes that are really namedtuples

Example:

class EmployeeRecord:
    def __init__(self, first_name, last_name, email, employee_number, australian_address):
        self.first_name = first_name
        self.last_name = last_name
        self.email = email
        self.employee_number = employee_number
        self.australian_address = australian_address


class RecordValidator:
    def __init__(self):
        pass

    @staticmethod
    def validate_employee_email(employee_record):
        return '@' in employee_record.email

    @staticmethod
    def validate_employee_australian_address(employee_record):
        return 'Australia' in employee_record.australian_address

    @staticmethod
    def validate_record(employee_record):
        return (self.validate_employee_email(employee_record) and
                self.validate_employee_australian_address(employee_record))

Why it’s bad:

This isn’t object oriented.

Writing code this way creates objects that are basically like namedtuples in Python, or structs in C – with some “utility” classes with functions that take that struct, read the data from the struct, and mutate that struct. This is essentially procedural programming using classes.

Objects are supposed to use and manage the data they have. This is just a missed opportunity in so many ways.

Solution:

class EmployeeRecord:
    def __init__(self, first_name, last_name, email, employee_number, australian_address):
        self.first_name = first_name
        self.last_name = last_name
        self.email = email
        self.employee_number = employee_number
        self.australian_address = australian_address

    def validate(self):
        return self._validate_email() and self._validate_australian_address()

    def _validate_email(self):
        return '@' in self.email

    def _validate_australian_address(self):
        return 'Australia' in self.australian_address

Also, read a book on object oriented programming. I highly recommend Practical Object-Oriented Design in Ruby by Sandi Metz.