Code smells, fifth part

By Detlef Wuppermann

In our first four parts of our series we discussed a small (Python) script. We went through the most obvious code smells and fixed them in every step.

from datetime import datetime, timedelta
import sys

import requests
from requests.auth import HTTPBasicAuth

from configuration import http_user_service_url, http_user_service_username, http_user_service_password,
                          userdata_filename, age_in_years

import csv

authentication = None
min_age = None
file_handler = None
writer = None

def setup():
    global authentication
    global min_age
    global writer
    global file_handler

    authentication = HTTPBasicAuth(http_user_service_username, http_user_service_password)

    min_age = datetime.now() - timedelta(days=age_in_years * 365)

    file_handler = open(userdata_filename, 'w')
    writer = csv.writer(file_handler)

def tear_down():
    file_handler.close()

def is_user_from_country(user):
    if user['country'] == "Spain":
        return True
    LOG.debug(f"User with id ${user['id']} is not from Spain.")
    return False

def is_user_old_enough(user):
    global min_age

    birth_date_str = user['birth_date']
    birth_date = datetime.fromisoformat(birth_date_str)
    is_old_enough = birth_date < min_age
    if not is_old_enough:
        LOG.debug(f"User with id ${user['id']} is not old enough and will be skipped.")
    return is_old_enough

def get_all_users():
    global authentication

    LOG.debug(f"Fetching data from user service with response code {response.code}.")
    response = requests.get(http_user_service_url, auth=authentication)

    if response.status_code != 200:
        LOG.error(f"Could not make a successful request to ${http_user_service_url}. The error message is {response.text}.")
        sys.exit(1)

    response_dict = response.json()
    all_users = response_dict['results']

def write_user(user, user_count):
    new_user = {
        "name": f"{user["title"]} {user["first_name"]} {user["last_name"]}",
        "street": user["street"],
        "city": f"{user["postal_code"]} {user["city"]}",
        "country": user["country"],
    }
    writer.writerow(user.keys())

def filter_users_of_legal_drinking_age():
    all_users = get_all_users()
    filtered_users_count = 0
    for user in all_users:
        if not is_old_enough(user):
            continue
        if not is_user_from_country(user):
            continue
       write_user(user, filtered_users_count)
       filtered_users_count = filtered_users_count + 1
    LOG.debug(f"{filtered_users_count} from {len(all_users)} total users found.")


if __main__ == main():
    setup()
    filter_users_of_legal_drinking_age()
    tear_down()

Before we move on, let’s get rid of some nitty grittiest, smaller things, but I would like to address them so that we do not stumble upon these issues in the next chapters.

Fix the naming of your variables and functions

I tried to explain, that it is important to name everything so that it gets clear on first sight what the name of a variable or function means exactly. So, my question would be: What exactly does “user” mean?

From the Wordpress documentation, there are different kinds of user, depending on their role.

  • Admin

A role “user” is not mentioned here, so you should not use it if there are more than only one role in your application.

So in this case we are talking about, let’s say, customers.

Also, some variables like min_age can be improved.

Typing

This is only important for languages without strict typing, like Python or PHP.

I often hear the argument that untyped languages are flexible, and typing only restricts the way you work.

Well, do we really want to be “flexible”? I mean in the sense that we do not decide what the code should do exactly and the functionality could change in the future? Good code tells you what it exactly does, which parameter functions take and what they return. There shouldn’t be “maybes” in your code, so be explicit and tell the future readers of your code!

In my opinion, Python is struggling from the time when typing was not an option. The fact that a function can take any argument that then will be provided via the “**kwargs” argument is a code smell by itself. I am, to this day, struggling with the interface that the requests lib implements. Way too often I try to find information about potential arguments and I end up using a search engine instead of using the documentation. !! TODO is there something like a Python Doc?

Documentation

Well, I said that we should write the code so that we don’t need documentation, didn’t I?

But there is also documentation that is expected in your code. You should add documentation for every function that you use (and if you use classes, also for them).

And maybe you also want to document every file, add some information about the license, the developers and how to find more information about this code.

I will add that, and I will see you in the next chapter.