For the last two weeks we’ve been analyzing a program, written by an archaeologist named Michael Troyer, that measures commute times. So far we’ve admired the way in which its logic is split up, and have talked about how its error handling could be improved. In this final discussion of the program we’ll look at how we can tidy up and significantly shorten the code that handles querying its database.
Shorter programs aren’t always better. Sometimes it’s pragmatic to be verbose today in order to make your code more understandable tomorrow. But if you can make your code shorter and more readable at the same time then you’ve got a recipe for a cake plus eating it type of situation.
(You may want to open the code in GitHub to reference while you read this post.)
Quick recap - Michael’s program has two commands. The first repeatedly queries the Google Maps API for the current commute time between two locations, and writes the results to a database. The second command reads the results back out of the database and calculates their average, range, and standard deviation.
Today we’re going to work on a function on Michael’s
Database class called
get_data is responsible for retrieving information about commute times from the program’s database, so that the program’s second command can analyze it. The function accepts arguments like
end_date, and uses these to filter down the data that it returns. Inside the function it constructs a SQL query using the arguments it receives, runs the query against the database, and returns the results.
You don’t need to know anything about the details of SQL in order to understand the
get_data function, but here’s an example of a SQL query constructed by
SELECT * FROM CommuteTimes WHERE Datetime > '2019-11-11' AND Origin = "3180 18th Street, San Francisco" AND Destination = "610 Townsend Street, San Francisco"
And here’s the
get_data function as it is written today. It’s perfectly correct and functioning, but it’s also an 80-line monster. Have a read of it. Before reading further, see if you can identify any unnecessarily verbose sections. Then see if you can rewrite it in a much pithier form. As we’ll see, I was able to get it down to around 23 lines, making it more readable in the process.
class Database: def get_data(self, start_date=None, end_date=None, specific_route=None): """ Return data from database between start_date and end_date. Optionally allow a specific route: (origin, destination) tuple. Returns a pandas dataframe. """ if specific_route: origin, destination = specific_route try: with sqlite3.connect(self.db_path) as con: if specific_route: if start_date: if end_date: # Start date and end date and specific route query = "\ SELECT * FROM CommuteTimes \ WHERE (\ Datetime > ? AND \ Datetime < ? AND \ Origin = ? AND \ Destination = ? \ )" params = (start_date, end_date, origin, destination,) else: # Start date only and specific route query = "\ SELECT * FROM CommuteTimes \ WHERE (\ Datetime > ? AND \ Origin = ? AND \ Destination = ? \ )" params = (start_date, origin, destination,) elif end_date: # End date only and specific route query = "\ SELECT * FROM CommuteTimes \ WHERE (\ Datetime < ? AND \ Origin = ? AND \ Destination = ? \ )" params=(end_date, origin, destination,) else: # No start or end date but specific route query = "\ SELECT * FROM CommuteTimes \ WHERE (\ Origin = ? AND \ Destination = ? \ )" params = (origin, destination,) else: if start_date: if end_date: # Start date and end date query = "SELECT * FROM CommuteTimes WHERE (Datetime > ? AND Datetime < ?)" params = (start_date, end_date,) else: # Start date only query = "SELECT * FROM CommuteTimes WHERE Datetime > ?" params = (start_date,) elif end_date: # End date only query = "SELECT * FROM CommuteTimes WHERE Datetime < ?" params = (end_date,) else: # No start or end date query = "SELECT * FROM CommuteTimes" params = None # EDITOR'S NOTE: don't worry about exacty how `pd.read_sql_query` # works. It's job is to query the database and format the results, # but the details of how and why aren't important. return pd.read_sql_query(query, con=con, parse_dates=['Datetime'], params=params) except Exception as e: print('Error getting data from database:', e)
get_data, I would first get rid of the
except block entirely, for all the reasons discussed in our previous episode. If
get_data throws an exception, it shouldn’t try to hide this fact by swallowing it. Instead, it should allow the code that called it to decide how to respond.
Second, I would try to reduce the repeated repetitive repetition throughout the function. When I read
get_data for the first time, I immediately noticed that the string
SELECT * From CommuteTimes was repeated 8 times. I also noticed that we have multiply-nested if-statements for every possible combination of the function’s arguments. For example, the function starts with the 3 blocks:
if specific_route => if start_date => if end_date. This prol-IF-eration (sorry) is the root cause of the code’s already extreme repetitiousness. What’s more, suppose that we wanted to add another filter variable into our SQL query (such as
time_of_day). We would need to add a fourth level of if-nesting to every block in our already precarious E-IF-fel Tower (sorry again). This would double the number of if-branches, to 16. This is madness.
After I got to the end of the function I didn’t immediately have a plan for how to tighten it up, but I knew that if I couldn’t then we were all doomed. I pondered a while. Then I noticed that none of the variables in the different if-branches depend on each other. By this I mean that the effect of an
if start_date block is always to simply add a
Datetime > ? condition to the
WHERE clause of the SQL statement. It doesn’t matter whether
specific_route is set. This means that we can examine every argument in its own, simple, un-nested if-statement. We can use these statements to build up a list of the different
WHERE filters that we need to apply, and then construct the SQL query dynamically at the end of the function.
Here’s what I mean:
class Database: def get_data(self, start_date=None, end_date=None, specific_route=None): """ Return data from database between start_date and end_date. Optionally allow a specific route: (origin, destination) tuple. Returns a pandas dataframe. """ if specific_route: origin, destination = specific_route filters =  if specific_route: filters.append(("Origin = ?", origin)) filters.append(("Destination = ?", destination)) if start_date: filters.append(("Datetime > ?", start_date)) if end_date: filters.append(("Datetime < ?", end_date)) query = "SELECT * FROM CommuteTimes" if len(filters) > 0: # [f for f in filters] is a Python "list # comprehension". It is shorthand for: # # l =  # for f in filters: # l.append(f) # # Here we use a list comprehension to # extract the first elements of the filter # pairs we just created, and use them to # construct our query. clauses = [f for f in filters] query += " WHERE " + " AND ".join(clauses) with sqlite3.connect(self.db_path) as con: return pd.read_sql_query(query, con=con, parse_dates=['Datetime'], params=[f for f in filters])
Now if we want to add a
time_of_day parameter, we no longer have to add 8 new if-statement branches. Instead, we can add a single extra 2 line if-statement and call it a day:
if time_of_day: filters.append(("TimeOfDay = ?", time_of_day))
That’s it. The pre-existing query-building code will magically take care of integrating the new filter into the query.
In order to apply this kind of reasoning to your own code, start by developing your sense of smell. Almost every day I write code that feels somehow off but that I don’t know how to improve. I usually finish up my attempt to the best of my ability, and then ask a co-worker for their opinion and advice. Sometimes they have some ideas, sometimes they don’t.
Developing a sense for when a piece of code can be improved is an important skill in itself, even when you aren’t immediately sure what to do about it. As you expand your toolbox of tricks and patterns, you’ll start to be able to fix more of the problems that you notice. Then you’ll start noticing more problems, then you’ll be able to fix them, then you’ll notice more problems, then… You’ll never be perfect or satisfied, but you will hopefully get paid more.
Until next week: