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?

\$\endgroup\$
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
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.