Code smells, second part
By Detlef Wuppermann
In part one of our series we discussed a small (Python) script. We went through the most obvious code smells and fixed them in the code below.
http_user_service_url = "https://api.coding-on-my-mind.com/users"
http_user_service_username = os.envviron.get("HTTP_USER_SERVICE_USERNAME")
http_user_service_password = os.envviron.get("HTTP_USER_SERVICE_PASSWORD")
age_in_years = 18
userdata_filename = "./clients.csv"
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
f = open(userdata_filename, 'w')
writer = csv.writer(f)
now = datetime.now()
min_age = now - timedelta(days=age_in_years * 365)
response = requests.get(http_user_service_url, auth=HTTPBasicAuth('u938453', 'Wxc6$78gfsgOi8kr'))
# print(response)
if response.status_code != 200:
print(f"Could not make a successful request to ${url}")
sys.exit(1)
response_dict = response.json()
all_users = response_dict['results']
counter = 0
for user in all_users:
birth_date_str = user['birth_date']
birth_date = datetime.fromisoformat(birth_date_str)
if birth_date < min_age:
if user['country'] == "Spain":
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"],
}
if counter == 0:
writer.writerow(new_user.keys())
writer.writerow(new_user.values())
counter = counter + 1
else:
print(f"User with id ${user['id']} is not from Spain")
else:
print(f"User with id ${user['id']} is not old enough and will be skipped")
f.close()
We improved the code a little bit in comparison to the code from our first part.
Use code linter
The next thing that we see is that the code still looks unstructured. One easy way to improve the code is to use so-called linters. Linters check apply best practices on your code which make them easier to read. Also, linters format the code in a way most IDEs do, which will lead to code that does not change whenever anybody decides to work on the code.
Use a logger instead of printing stuff to the console
We still have one comment left that contains a print statement. What you often can see are print statements that are used for debugging the application. There are multiple ways to get rid of these print statements, one of these are using a logger. The advantages are that you can use a logger in the code that runs in production when you maybe do not want to print out some statements to the console but into a file.
In our case we replaced all print-statements by log commands. We differentiated between multiple level of logging, the debug level for the print statement that was commented out, the info level for information that is useful when running your script, and the error level in case of unexpected errors. Especially in the error case you want to use log messages that help you with reproducing the root case for the error, so keep the message meaningful.
Don’t use abbrs where not needed, instead, use descriptive names
You might guess what I mean with abbrs. But what when an abbreviation is not commonly known, especially for new colleagues? This would make reading and understanding the code way harder, or even impossible. Also, you see the line where we make the request? The variable “authentication” is not cropped, so now everybody knows what it means, but the requests library uses “auth” which could also stand for “authorization” (although this would be very unlikely in this case). So, you see that using meaningful variable und function names should be the standard, but even for libraries this is not always the case.