patternpythonMinor
"Day-of-the-week-finder"
Viewed 0 times
thedayfinderweek
Problem
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
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 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()Solution
todayFirst 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_betweenIt 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_dtI 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.Code Snippets
today = datetime.date.today()
today = today.strftime("%m/%d/%Y")
today = datetime.datetime.strptime(today, "%m/%d/%Y")today = datetime.datetime.today()def days_between(d1, d2):
return (d2 - d1).days
def days_from_today(date):
return days_between(date, today)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))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))Context
StackExchange Code Review Q#109546, answer score: 2
Revisions (0)
No revisions yet.