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!