Celestial age calculator

Question

Our solar system has 8 planets, including earth. Our calendar has some very confusing concepts such as leap years, which I still don't fully understand. In fact, since where I live, we use a different solar calendar that is different from the Gregorian reforms of the Julian Western calendar, I think I'm born in March but I should be born in February! Anyways, I attempted to create a simple program that calculates any given age, subtracting the leap days of each four years, in any planet of our solar system except Pluto which I believe is not a planet. I'm not sure about it.

def calculate_days(age):
    leap_days = 0 #holds number of leap days
    leap_years_list = [i for i in range(age) if i % 4 == 0] #holds the essence of leap years

    for j in leap_years_list:
        if j % 4 == 0 or j % 400 == 0 and j % 100 != 0:
            leap_days += 1 #if leap years are divisible by four and not divisible by a hundred, add to leap days

    return (age * 365) - leap_days


def calculate_celestial_age(planet, age):
    number_of_days = calculate_days(age) #calculate the days in age

    assert str.islower(planet), "Planet name must be entered in lower case e.g. 'mercury'" #if planet name is in lowercase, give error

    days_in_year = {"mercury": 88, "venus": 224, "earth": 365, "mars": 687, "jupiter": 4332, "saturn": 10759, "uranus": 30688,
                    "neptune": 60182} #days in each planet's year

    if planet == "mercury":
        return number_of_days / days_in_year["mercury"]
    elif planet == "venus":
        return number_of_days / days_in_year["venus"]
    elif planet == "earth":
        return number_of_days / days_in_year["earth"]
    elif planet == "mars":
        return number_of_days / days_in_year["mars"]
    elif planet == "jupiter":
        return number_of_days / days_in_year["jupiter"]
    elif planet == "saturn":
        return number_of_days / days_in_year["saturn"]
    elif planet == "uranus":
        return number_of_days / days_in_year["uranus"]
    elif planet == "neptune":
        return number_of_days / days_in_year["neptune"]
    else:
        raise Exception("Unknown Planet! Are you sure you've enter a planet?")


age = 23

planets = ["mercury", "venus", "earth", "mars", "jupiter", "saturn", "uranus", "neptune"]

for planet in planets:
    print("{0}: {1}".format(planet, calculate_celestial_age(planet, age)))

For my age, 23, I got these results:

mercury: 95.3409090909091

venus: 37.455357142857146

earth: 22.986301369863014

mars: 12.212518195050945

jupiter: 1.9367497691597415

saturn: 0.7798122502091273

uranus: 0.2733967674661105

neptune: 0.1394104549533083

According to this, my results are accurate.

Answer 1

Simplify the logic

It seems like:

if planet == "mercury":
    return number_of_days / days_in_year["mercury"]
elif planet == "venus":
    return number_of_days / days_in_year["venus"]
elif planet == "earth":
    return number_of_days / days_in_year["earth"]
elif planet == "mars":
    return number_of_days / days_in_year["mars"]
elif planet == "jupiter":
    return number_of_days / days_in_year["jupiter"]
elif planet == "saturn":
    return number_of_days / days_in_year["saturn"]
elif planet == "uranus":
    return number_of_days / days_in_year["uranus"]
elif planet == "neptune":
    return number_of_days / days_in_year["neptune"]

can be easily rewritten:

if planet in days_in_year:
    return number_of_days / days_in_year[planet]
raise Exception("Unknown Planet! Are you sure you've enter a planet?")

Also, one could consider keeping it even more simple and use the fact that:

return number_of_days / days_in_year[planet]

will throw the relevant exception for an invalid value.

Do not repeat yourself

You can try to avoid duplicated values and have a single source of information. In your case, the list of planet is indirectly hardcoded twice :

days_in_year = {"mercury": 88, "venus": 224, "earth": 365, "mars": 687, "jupiter": 4332, "saturn": 10759, "uranus": 30688,
                "neptune": 60182} #days in each planet's year

and

planets = ["mercury", "venus", "earth", "mars", "jupiter", "saturn", "uranus", "neptune"]

Maybe you could define a constant like DAYS_IN_YEAR_PER_PLANET (corresponding to your actual days_in_year dictionnary) and use it where you are using the list of the planets (for planet in DAYS_IN_YEAR_PER_PLANET: for instance).

Do not perform more operations than required

number_of_days = calculate_days(age) is computed for every planet in the list. It would be better from a performance point of view to feed the function a number of days.

Useless list (or useless test)

You are creating a list with values divisible by 4. Then you iterate over it and check if the value is divisible by 4. It seems like a waste of effort. Let's get rid of the list creation. Code is based on the initial version of your code.

def calculate_days(age):
    leap_days = 0 #holds number of leap days

    for j in range(age):
        if j % 4 == 0 and j % 100 != 0:
            leap_days += 1 #if leap years are divisible by four and not divisible by a hundred, add to leap days

    return (age * 365) - leap_days

Also, this can somehow be written by abusing the generator expression and the sum builtin:

def calculate_days(age):
    leap_days = sum(1
                    for j in range(age)
                    if j % 4 == 0 and j % 100 != 0)
    return (age * 365) - leap_days

Also, you could find mathematical expressions to compute the number of leap days in a more efficient way (constant time instead of linear time) but I'll keep this out of the review.