I am refactoring a function that calculates how many days to add to a process based on destination country or state.

The original function called the database twice, even though the database table stored four values. My thought is to remove the database calls and use a config constant, or simply put the relevant ID numbers right in the code. Which of these methods do you think is best? Or should I do something different?

# Method 1
def get_transit_time(country, state=None):
    if country.id in [1, 221]:  # US, Puerto Rico
        days = 7
    elif country.id in [13, 138]:  # Australia, New Zealand
        days = 10
    elif country.id == 37:  # Canada
        days = 12
        days = 14  # all others

    if state and state.id in [2, 15]:  # Hawaii, Alaska
        days = 10

    return days

# method 2 (allows for growth via config constant)
    {'country_id': 1, 'delay': 7},
    {'country_id': 221, 'delay': 7},
    {'country_id': 13, 'delay': 10},
    {'country_id': 138, 'delay': 10},
    {'country_id': 37, 'delay': 12}

    {'state_id': 2, 'delay': 10},
    {'state_id': 10, 'delay': 10}

def get_card_transit_time(country, state=None):
    days = 14  # default if no result found

    for country_config in SEND_DELAY_COUNTRIES:
        if country_config['country_id'] == country.id:
            days = country_config['delay']

    if state:
        for state_config in SEND_DELAY_STATES:
            if state_config['state_id'] == state.id:
                days = state_config['delay']

    return days
  • \$\begingroup\$ It should go in the same place that the country/state_name to country/state_id mapping is stored. When country/state information needs to be changed, it's best if you only need to make changes in one place. \$\endgroup\$ – RootTwo Nov 2 '20 at 18:19
  • \$\begingroup\$ First of all, if you have hardcoded numbers and have to add comments that it means Puerto Rico, you are probably doing something wrong. Define an enum for readability. \$\endgroup\$ – Zachary Vance Nov 4 '20 at 1:50

Your Answer

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

Browse other questions tagged or ask your own question.