Celestial age calculator

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.

1 Answer

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.

• How can I only use values, or only the keys, of a dictionary? – CodesInTheValley Jul 13 '16 at 15:25
• By default, it loops over keys. You have ways to iterate over (key, values) or over (values). More information here : stackoverflow.com/questions/3294889/… . – SylvainD Jul 13 '16 at 15:26
• @cat This comment is more relevant to OP's question than to my answer. – SylvainD Jul 13 '16 at 21:15
• I didn't notice that in the original code, now I see why it's an Exception and not a ValueError or something – cat Jul 13 '16 at 21:22