3
\$\begingroup\$

I'm working on a function that returns a specific set of times for a given timezone and a given frequency; day, week, month. For any given timezone and a frequency the function will return the unixtime hour start and unixtime hour end of the last full frequency given. For example, the last full day, the last full 7 days or the last full month.

import calendar
import pytz
import time
from datetime import datetime, timedelta, time
def get_international_dates(timezone, frequency):
 tz = pytz.timezone(timezone)
 today = datetime.now(tz).date()
 midnight = tz.localize(datetime.combine(today, time(0, 0)), is_dst=True)
 last_full_day_midnight = int(midnight.astimezone(pytz.utc).strftime("%s"))
 if frequency == 'd':
 day_end = last_full_day_midnight - 3600
 day_start = last_full_day_midnight - (3600 * 24)
 prev_day_end = day_end - (3600 * 24)
 prev_day_start = day_start - (3600 * 24)
 return {'start': day_start, 'end': day_end, 'prev_start': prev_day_start, 'prev_end': prev_day_end}
 if frequency == 'w':
 week_end = last_full_day_midnight - 3600
 week_start = last_full_day_midnight - (3600 * 24 * 7)
 prev_week_end = week_end - (3600 * 24 * 7)
 prev_week_start = week_start - (3600 * 24 * 7)
 return {'start': week_start, 'end': week_end, 'prev_start': prev_week_start, 'prev_end': prev_week_end}
 if frequency == 'm':
 month_length = calendar.monthrange(today.year, today.month - 1)
 month_end = last_full_day_midnight - 3600
 month_start = last_full_day_midnight - (3600 * 24 * month_length[1])
 prev_month_end = month_end - (3600 * 24 * month_length[1])
 prev_month_start = month_start - (3600 * 24 * month_length[1])
 return {'start': month_start, 'end': month_end, 'prev_start': prev_month_start, 'prev_end': prev_month_end}

This function works as it should, but it seems pretty messy and un-pythonic. What should I do to make this more clean, concise and pythonic?

200_success
146k22 gold badges190 silver badges479 bronze badges
asked Jun 30, 2016 at 20:06
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Some small things that jump out to me:

  • It looks like a lot of duplication in each if check

  • There are a lot of magic numbers

  • Those later if checks should probably be elif, unless it's possible to satisfy several of them in one pass

  • Day/week/month end are identical

  • You should probably have a single return at the bottom - the things that fill it are the same each time, so just use the checks to determine what gets returned. Then you only need to change the formatting in one place if you decide to later.

I think I'd prefer to write this with a dictionary mapping adjustments to each type of frequency. After doing this, calculating the return values for each category can be done by the same code:

adjustment = {
 end: {#dict for adjustments for each frequency},
 start: {#dict for adjustments for each frequency},
 prev_end: {#dict for adjustments for each frequency},
 prev_start: {#dict for adjustments for each frequency}
}
def get_international_dates(timezone, frequency):
 tz = pytz.timezone(timezone)
 today = datetime.now(tz).date()
 midnight = tz.localize(datetime.combine(today, time(0, 0)), is_dst=True)
 last_full_day_midnight = int(midnight.astimezone(pytz.utc).strftime("%s")) 
 end = last_full_day_midnight - adjustment[end][frequency]
 start = end - adjustment[start][frequency]
 prev_end = end - adjustment[prev_end][frequency]
 prev_start = start - adjustment[prev_start][frequency]
 return end, start, prev_end, prev_start
answered Jun 30, 2016 at 20:45
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.