5
\$\begingroup\$

I am crazy about needing to calculate my work shifts for my part-time job while attending University. This is because I cannot work more than 20 hours a week.

Yes, this code is crap, I wrote it in 20 minutes... my brain operates kinda like a pachinko machine, and whatever it writes for stupidly annoying scripts like this it just writes, screw quality. Don't ding me for that, please, I just haven't had enough coffee or sleep lately to write better code.

So, I wrote a Python script that lets me do one of two things:

  1. Process my 'normal' work schedule, which is hardcoded as the 'default' schedule
  2. Process the first argument (which should be a dict containing the name of the day of the week, and contain a list of lists which contain the start and end time for each shift, using 24hr time format), passed into the program at runtime as an argument.

Regardless of these cases, it will always return something like:

Total Weekly Hours: 15.0

(as an example, using the 'default' hardcoded schedule).

When executing as a shell command, or with the python executable, you have to put the "schedule" dict in quotes. This is an example of execution of the file and passing in a schedule where I have to take on additional hours (on the command line, so this would be executed verbatim):

./workhours.py "{'Monday': [['09:00', '13:00']], 'Tuesday': [['09:00', '10:30'], ['17:00', '19:00']], 'Wednesday': [['09:00', '11:00']], 'Thursday': [['13:00', '15:00']], 'Friday': [['10:00', '17:00']]}"

As long as the first argument is an entire dict, as a string so it's parsed right by the application, it seems to work fine, and gives me correct values (for example, the last example would return a value of 18.5 hours which is accurate).

While this works, I'd like suggestions for how I could improve this. It's a quickly-thrown-together script that does what I want, but suggestions for improvement are definitely accepted.


This is the code:

workhours.py:

#!/usr/bin/python
import sys
import os
from ast import literal_eval
from datetime import datetime, timedelta

_TIME_FORMAT = '%H:%M'

def set_timeformat(self, formatstr=_TIME_FORMAT):
    self._TIME_FORMAT = formatstr

_DEFAULT_SCHEDULE = {
    'Sunday': None,
    'Monday': [['09:00', '13:00']],
    'Tuesday': None,
    'Wednesday': [['09:00', '11:00']],
    'Thursday': [['13:00', '15:00']],
    'Friday': [['10:00', '17:00']],
    'Saturday': None
}

def _calc_shift_hours(shift):
    tdelta = datetime.strptime(shift[1], _TIME_FORMAT) - datetime.strptime(shift[0], _TIME_FORMAT)

    if tdelta.days < 0:
        tdelta = timedelta(days=0, seconds=tdelta.seconds, microseconds=tdelta.microseconds)

    return tdelta.seconds / 3600.0


def _calc_workhours(sched=_DEFAULT_SCHEDULE):
    hours = {
        'Sunday': 0,
        'Monday': 0,
        'Tuesday': 0,
        'Wednesday': 0,
        'Thursday': 0,
        'Friday': 0,
        'Saturday': 0
    }

    totalhours = 0

    for day in sched.iterkeys():
        if sched[day] is None:
            pass
        else:
            # noinspection PyTypeChecker
            for shift in sched[day]:
                hours[day] += _calc_shift_hours(shift)
            totalhours += hours[day]

    os.system('clear')
    print "Total Weekly Hours: %s" % totalhours

if __name__ == '__main__':
    try:
        dictarg = literal_eval(sys.argv[1])
        if isinstance(dictarg, dict):
            schedule = dictarg
        else:
            schedule = _DEFAULT_SCHEDULE
    except IndexError:
        schedule = _DEFAULT_SCHEDULE

    _calc_workhours(schedule)
\$\endgroup\$
5
\$\begingroup\$

I would improve the way you calculate total hours - you don't actually need to keep track of how much hours per day of a week you can work and do the calculations in one go on the "schedule" object with the sum() built-in function:

def _calc_workhours(sched=_DEFAULT_SCHEDULE):
    """Returns total work hours from a schedule."""
    return sum(_calc_shift_hours(shift) 
               for shifts in sched.values() if shifts 
               for shift in shifts)

Note that, to define the "scope" of a function more clearly and set the clear boundaries, it returns work hours instead of printing it and executing an os.system() call (which I consider a side effect). Then, to print the result, edit the main block of the program and replace:

_calc_workhours(schedule)

with:

total_hours = _calc_workhours(schedule)
os.system('cls||clear')
print("Total Weekly Hours: %s" % total_hours)

Other ideas:

  • use argparse module instead of reading from sys.argv directly - it will be cleaner overall, you would have a simple and clear way to define a "--help" mode for your command-line program. Would be easier to set the default value for the schedule
  • instead of parsing the schedule with literal_eval() from the command-line consider accepting a JSON file with your schedule object which you would load with json.load()
  • in any case, reading and parsing the schedule should also be extracted into a separate function
  • we can also refactor _calc_shift_hours() a bit and instead of accepting the shift list, accept two arguments, shift_start and shift_end, a bit more explicit and cleaner:

    def _calc_shift_hours(shift_start, shift_end):
        """Returns a number of hours per shift."""
        tdelta = datetime.strptime(shift_end, _TIME_FORMAT) - datetime.strptime(shift_start, _TIME_FORMAT)
        # ...
    
    def _calc_workhours(sched=_DEFAULT_SCHEDULE):
        """Returns total work hours from a schedule."""
        return sum(_calc_shift_hours(shift_start, shift_end)
                   for shifts in sched.values() if shifts
                   for shift_start, shift_end in shifts)
    
  • print() function vs print statement, .values() vs itervalues() for Python 3 compatibility

  • missing docstrings (I've added them in the snippets above - they still can/should be more detailed)
  • the way you set the default schedule at the moment can be improved so that, if a default schedule is used, you know about it, something like:

    try:
        dictarg = literal_eval(sys.argv[1])
        if isinstance(dictarg, dict):
            schedule = dictarg
        else:
            print("Invalid schedule object provided. Using the default schedule.")
            schedule = _DEFAULT_SCHEDULE
    except IndexError:
        print("No schedule provided. Using the default schedule.")
        schedule = _DEFAULT_SCHEDULE
    
  • if literal_eval() cannot evaluate a string - there will be ValueError thrown - currently it is not handled properly. This again, can be beautifully solved by argparse, or/and if you switch to having schedule in JSON

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