"Day-of-the-week-finder"

Question

I'm new to Python/Programming and wrote this simple "Day of the week/Holiday finder" script. Ideally I'd use something so I can have a little widget on my desktop, but right now I'm looking for any and all suggestions to improve my code?

I'm importing this holiday module

import datetime
import holidays
today = datetime.date.today()
today = today.strftime("%m/%d/%Y")
today = datetime.datetime.strptime(today, "%m/%d/%Y")
def days_between(d1):
 d2 = today
 return (d2 - d1).days
def find_dt(input_dt):
 print(input_dt)
 dt1 = datetime.datetime.strptime(input_dt, "%m/%d/%Y")
 dt2 = dt1.strftime('%A')
 if days_between(dt1) == 0:
 print("Today IS {d}.".format(d=dt2))
 elif days_between(dt1) <= 1:
 print("It'll be a {d}.".format(d=dt2))
 else:
 print("It was a {d}.".format(d=dt2))
 us_holidays = holidays.UnitedStates()
 us_h = us_holidays.get(input_dt)
 if us_h is None and days_between(dt1) == 0:
 print("Today is not a holiday.")
 elif us_h is None and days_between(dt1) <= 1:
 print("It won't be a holiday.")
 elif us_h is None and days_between(dt1) >= 1:
 print("It wasn't a holiday.")
 elif days_between(dt1) == 0:
 print("Today is {h}.".format(h=us_h))
 elif days_between(dt1) <= 1:
 print("It'll be {h}.".format(h=us_h))
 else:
 print("It was {h}.".format(h=us_h))
def main():
 d = input('Type a date using MM/DD/YYYY: ')
 # d = '12/25/2015'
 find_dt(d)
main()

• SuperBiasedMan: Welcome to Code Review! Are you importing this holiday package? If so please edit the link into your post.

Answer 1

today

First of all, I'm super confused as to why you're doing

today = datetime.date.today()
today = today.strftime("%m/%d/%Y")
today = datetime.datetime.strptime(today, "%m/%d/%Y")

It seems like you'd be fine if you just did

today = datetime.datetime.today()

Additionally, constants (such as today) are traditionally named in ALL_CAPS.

days_between

It is very counter-intuitive to me that this only takes a single argument. I think writing it this way

def days_between(d1, d2):
 return (d2 - d1).days
def days_from_today(date):
 return days_between(date, today)

makes much more sense.

find_dt

I don't like everything that is going on here. First of all, I would argue that it shouldn't be the job of find_dt to convert an input string to a datetime object - the caller should be responsible for that.

Next, you're doing two distinct things. One is displaying the day of the week for the given date, and the other is displaying information about that day being a US holiday. These should probably be distinct.

def day_of_week(date):
 date_string = date.strftime("%A")
 from_today = days_from_today(date)
 if from_today == 0:
 info_string = "Today IS {}"
 elif from_today <= 1:
 info_string = "It will be a {}"
 else:
 info_string = "It was a {}"
 print(info_string.format(date_string))

A few notes about this. I didn't call days_from-today every time - that's pretty wasteful. Additionally, because each one does basically the same thing, instead of repeating the print I create the appropriate string and then just print into that at the end.

Next, holidays.

def holiday_status(date):
 us_holidays = holidays.UnitedStates()
 us_h = us_holidays.get(date)
 from_today = days_from_today(date)
 if us_h is None:
 if from_today == 0:
 print("Today is not a holiday")
 elif from_today <= 1:
 print("It won't be a holiday")
 else:
 print("It wasn't a holiday")
 else:
 if from_today == 0:
 info_string = "Today is {}"
 elif from_today <= 1:
 info_string = "It'll be {}"
 else:
 info_string = "It was {}"
 print(info_string.format(us_h))

Notice that I used some similar strategies as with day_of_week. I also broke up your if statements a little more - while some people think nesting is bad, I think in this case it makes it much more clear what the conditions are.

__main__

You should have a block like this

if __name__ == '__main__':
 main()

at the bottom of your file. Then it is import safe.

Locale

While right now you assume the US, it is conceivable that your users may be from other locales. You probably want something like this (the specifics of this depend on your Python version, but this is a rough approximation)

LOCALE_ENUM = enum("US UK FR JP ...")
LOCALES = {
 LOCALE_ENUM.US: holidays.UnitedStates(),
 LOCALE_ENUM.UK: holidays.UnitedKingdom(),
 ...
}

and then to define both find_dt and holiday_status to take a LOCALE_ENUM instance and use it to determine what locale's holidays they should use.

• deez: Thanks @Dannnno. Took me a little while, but I got it working using your suggestions.. I'm curious about the reasoning behind creating to functions for days_between (days_between and days_from_today).
• Dan Oberlam: @deez What specifically do you mean?
• deez: Why couldn't you just wrap that logic into 1 function?
• Dan Oberlam: You could. days_from_today is completely superfluous - however you seemed to want to be able to have that behavior so I added it. Does that answer your question?