Emma found a function called `get_mileage_per_year`. The purpose of the function is to apply some business rules around travel expenses, and while I'm sure it does that… it also makes some choices.

``````def get_mileage_per_year(self):
# Mileage not set yet
if not self.mileage_per_day:
return 0

weekly_miles = self.mileage_per_day

extra_miles = 60 - self.mileage_per_day if self.purposes == ["special"] else 25

# Add in extra miles for 'special' purpose is selected

weekly_miles += extra_miles if "special" in self.purposes else 0

#   where average daily miles are given by the weekly miles variable
annual_miles = (weekly_miles * 365) + 2000
if "special" in self.purposes:
annual_miles = max(annual_miles, 24000)

# Return annual miles bound to range 4,000 < mileage < 50,000
mileage = int(min(max(annual_miles, 4000), 50000))
return mileage
``````

We start by checking if the `mileage_per_day` field has a value, which isn't a bad start. But the very next line is an incredible case of bad variable names: `weekly_miles = self.mileage_per_day`. Only one of these can be correct.

After that we have a bit of magic number based on whether the `purpose` was `special`. Which, the fact that they're comparing `self.purposes` against an array and not doing some sort of contains check implies that this is when the only purpose was "special", which maybe is correct?

Well, the next line does a contains check to decide if `extra_miles` should be applied. So I think the line above is probably correct- if the only purpose was "special", we use one rule, if "special" was just one of the purposes,we use a different rule.

Then we get to the perfect line of bad code:

``````    #   where average daily miles are given by the weekly miles variable
annual_miles = (weekly_miles * 365) + 2000
``````

At least there's a comment explaining that `weekly_miles` doesn't contain what it's name says. The 365 makes sense, but then we've got another magic number getting slapped in there.

The final few lines are more magic-number based range checks- if there were any "special" purposes, we set the floor at 24,000 miles. Then we clamp the whole thing to a range of 4,000-50,000 miles. And we throw in a bonus `int()` conversion, which the data should already be integers, but hey, it is Python, so we can't always guarantee that.

This code falls into a category that I call "obsessively requirements driven", in that someone had a requirement for how this calculation worked. The requirement probably was originally defined as a formula in someone's spreadsheet. It was then handed off to a developer who implemented exactly that requirement, with no thought at all to how to express this in code so that it would be maintainable.

They had a formula. They had a Python file. They put the formula in the Python file. Next requirement.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!