How to 'clean' the code?

Hello,

I have just completed the Time Calculator task from Python Course.

It is working ok, but I realise that it could be neater, however my attempts to “clean” it, return error:

Traceback (most recent call last):
  File "/Users/rafallunitz/PycharmProjects/Time_Calculator.py/venv/gowno.py", line 84, in <module>
    add_time("9:15 AM", "1:30")
  File "/Users/rafallunitz/PycharmProjects/Time_Calculator.py/venv/gowno.py", line 42, in add_time
    time_conversion()
  File "/Users/rafallunitz/PycharmProjects/Time_Calculator.py/venv/gowno.py", line 34, in time_conversion
    if is_afternoon == True:
UnboundLocalError: local variable 'is_afternoon' referenced before assignment

original code (before cleaning):

def add_time(start, duration, day_of_the_week=False):
    weekdays = ['monday', 'tuesday', 'wednesday', 'thursday', 'friday', 'saturday', 'sunday']
    is_afternoon = False
    start_details = start.split()
    [start_hour, start_minute] = start_details[0].split(':')
    start_hour = int(start_hour)
    start_minute = int(start_minute)
    if start_details.index(max(start_details)) == 1:
        am_or_pm = start_details[1]
    else:
        am_or_pm = None

    if am_or_pm == "PM":
        start_hour += 12
    else:
        start_hour = start_hour
    duration_hours = int(duration.split(":")[0])
    duration_minutes = int(duration.split(":")[1])
    total_time = (start_hour + duration_hours) * 60 + (start_minute + duration_minutes)
    hour_time_converted = total_time // 60
    hour_time_displayed = hour_time_converted
    if hour_time_converted == 12:
        is_afternoon = True
    if hour_time_converted >= 13:
        is_afternoon = True
        hour_time_displayed = hour_time_converted - 12
    end_time_minutes = total_time % 60
    if end_time_minutes < 10:
        end_time_minutes = str("0") + str(end_time_minutes)
    if is_afternoon == True:
        end_time = str(hour_time_displayed) + ":" + str(end_time_minutes) + str(" PM")
    else:
        end_time = str(hour_time_converted) + ":" + str(end_time_minutes) +str(" AM")
    end_time_minutes = total_time % 60
    if end_time_minutes < 10:
        end_time_minutes = str("0") + str(end_time_minutes)
    final_output = end_time
    if day_of_the_week:
        final_output = end_time + ", " + day_of_the_week.capitalize()
    else:
        final_output = end_time

    # next day case
    if total_time > 1440 and total_time < 2880:
        total_time = total_time - 1440
        hour_time_converted = total_time // 60
        if hour_time_converted == 12:
            is_afternoon = True
        if hour_time_converted >= 13:
            is_afternoon = True
            hour_time_displayed = hour_time_converted - 12
        else:
            is_afternoon = False
            hour_time_displayed = hour_time_converted
        end_time_minutes = total_time % 60
        if end_time_minutes < 10:
            end_time_minutes = str("0") + str(end_time_minutes)
        if is_afternoon == True:
            end_time = str(hour_time_displayed) + ":" + str(end_time_minutes) + str(" PM")
        else:
            end_time = str(hour_time_converted) + ":" + str(end_time_minutes) + str(" AM")
        final_output = end_time + str(" (next day)")
        if day_of_the_week:
            day_of_the_week = day_of_the_week.strip().lower()
            final_day = (weekdays.index(day_of_the_week) + 1 % 7)
            current_day = weekdays[final_day]
            final_output = end_time + ", " + current_day.title() + str(" (next day)")
        else:
            final_output = end_time + str(" (next day)")


    # n days later case
    if total_time > 2880:
        outstanding_days = total_time // 1440
        total_time = total_time % 1440
        hour_time_converted = total_time // 60


        if hour_time_converted == 12:
            is_afternoon = True
        if hour_time_converted >= 13:
            is_afternoon = True
            hour_time_displayed = hour_time_converted - 12
        else:
            is_afternoon = False
            hour_time_displayed = hour_time_converted
        if hour_time_converted == 0:
            hour_time_displayed = 12
        end_time_minutes = total_time % 60
        if end_time_minutes < 10:
            end_time_minutes = str("0") + str(end_time_minutes)
        if is_afternoon == True:
            end_time = str(hour_time_displayed) + ":" + str(end_time_minutes) + str(" PM")
        else:
            end_time = str(hour_time_displayed) + ":" + str(end_time_minutes) + str(" AM")
        final_output = end_time + " (" + str(outstanding_days) + " days later)"
        if day_of_the_week:
            day_of_the_week = day_of_the_week.strip().lower()
            final_day = ((weekdays.index(day_of_the_week) + outstanding_days) % 7)
            current_day = weekdays[final_day]
            final_output = end_time + ", " + current_day.title() + " (" + str(outstanding_days) + " days later)"
        else:
            final_output = end_time + " (" + str(outstanding_days) + " days later)"


    return(final_output)
    print(final_output)

add_time("9:15 PM", "5:30")



I assumed that part of the code which is the same in each case, can be put inside the function (def time_conversion()), which is included inside the main function (add_time).

Code with time_conversion function added:

def add_time(start, duration, day_of_the_week=False):
    weekdays = ['monday', 'tuesday', 'wednesday', 'thursday', 'friday', 'saturday', 'sunday']
    is_afternoon = False
    start_details = start.split()
    [start_hour, start_minute] = start_details[0].split(':')
    start_hour = int(start_hour)
    start_minute = int(start_minute)
    if start_details.index(max(start_details)) == 1:
        am_or_pm = start_details[1]
    else:
        am_or_pm = None

    if am_or_pm == "PM":
        start_hour += 12
    else:
        start_hour = start_hour
    duration_hours = int(duration.split(":")[0])
    duration_minutes = int(duration.split(":")[1])
    total_time = (start_hour + duration_hours) * 60 + (start_minute + duration_minutes)
    hour_time_converted = total_time // 60
    hour_time_displayed = hour_time_converted

    def time_conversion():
        if hour_time_converted == 12:
            is_afternoon = True
        if hour_time_converted >= 13:
            is_afternoon = True
            hour_time_displayed = hour_time_converted - 12
        else:
            hour_time_displayed = hour_time_converted
        end_time_minutes = total_time % 60
        if end_time_minutes < 10:
            end_time_minutes = str("0") + str(end_time_minutes)
        if is_afternoon == True:
            end_time = str(hour_time_displayed) + ":" + str(end_time_minutes) + str(" PM")
        else:
            end_time = str(hour_time_converted) + ":" + str(end_time_minutes) + str(" AM")
        end_time_minutes = total_time % 60
        if end_time_minutes < 10:
            end_time_minutes = str("0") + str(end_time_minutes)
        return end_time
    time_conversion()
    final_output = end_time
    if day_of_the_week:
        final_output = end_time + ", " + day_of_the_week.capitalize()
    else:
        final_output = end_time

    # next day case
    if total_time > 1440 and total_time < 2880:
        total_time = total_time - 1440
        hour_time_converted = total_time // 60
        time_conversion()
        final_output = end_time + str(" (next day)")
        if day_of_the_week:
            day_of_the_week = day_of_the_week.strip().lower()
            final_day = (weekdays.index(day_of_the_week) + 1 % 7)
            current_day = weekdays[final_day]
            final_output = end_time + ", " + current_day.title() + str(" (next day)")
        else:
            final_output = end_time + str(" (next day)")


    # n days later case
    if total_time > 2880:
        outstanding_days = total_time // 1440
        total_time = total_time % 1440
        hour_time_converted = total_time // 60

        time_conversion()
        final_output = end_time + " (" + str(outstanding_days) + " days later)"
        if day_of_the_week:
            day_of_the_week = day_of_the_week.strip().lower()
            final_day = ((weekdays.index(day_of_the_week) + outstanding_days) % 7)
            current_day = weekdays[final_day]
            final_output = end_time + ", " + current_day.title() + " (" + str(outstanding_days) + " days later)"
        else:
            final_output = end_time + " (" + str(outstanding_days) + " days later)"



    print(final_output)

add_time("9:15 AM", "1:30")




Where am I wrong?

Thanks in advance for feedback and please let me know if you have any additional questions.

is_afternoon is not always defined inside the inner function. It’s defined in the outer function, but because you’re assigning to somewhere in the inner function, python won’t look outside the local scope. This is subtle and confusing. There are two ways to fix it. First, you can use the nonlocal statement to tell python that you want the inside is_afternoon to be the same as the outside is_afternoon. Second, and better, is to avoid the problem entirely by making your functions self-contained. If you make time_conversion an entirely separate function that takes arguments, returns a value, and doesn’t try to change anything outside of itself, everything will be a lot simpler.

More about scoping rules

1 Like