2
\$\begingroup\$

I just wrote this script that calculates the time difference between two times. It's going to be used in a larger program that will store the returned value in a database.

I'm mainly wondering if I've used docstrings, comments and try/raise/except correctly since it's my first time really using them.

def time_difference(time_start, time_end):
    '''Calculate the difference between two times on the same date.

    Args:
        time_start: the time to use as a starting point
        time_end: the time to use as an end point

    Returns:
        the difference between time_start and time_end. For example:

        >>> time_difference(1500, 1600)
        60

        >>> time_difference(1100, 1310)
        130

    Raises:
        TypeError: if time_start and time_end is not a string
        ValueError: if time_start and time_end is below 0 or above 2400
    '''

    try:
        if not isinstance(time_start, str):
            raise TypeError('time_start has to be a string!')
        elif not isinstance(time_end, str):
            raise TypeError('time_end has to be a string!')
        elif int(time_start) > 2400 or int(time_end) < 0:
            raise ValueError('time_start nor time_end can be below 0 or above 2400!')
        elif int(time_start) > int(time_end):
            raise ValueError('time_start cannot be higher than time_end!')
    except (TypeError, ValueError):
        raise

    # Convert time_start and time_end to necessary variables 
    # for the calculation.
    hours_begin = time_start[0] + time_start[1]
    minutes_begin = time_start[2] + time_start[3]
    hours_end = time_end[0] + time_end[1]
    minutes_end = time_end[2] + time_end[3]

    # Calculates the difference.
    minutes = ((int(hours_end) - int(hours_begin)) * 60 + \
        int(minutes_end) - int(minutes_begin))

    return(minutes)
\$\endgroup\$
6
\$\begingroup\$

Both the function name and the docstring are clear and informative. Good job there! However, there are a couple of typos in the examples. The function requires strings as input but the examples use ints. For example time_difference(1100, 1310) should be time_difference('1100', '1310'). A tool like doctest can help you check for mistakes like this automatically.

Generally people don't check the types of arguments in python. You are right to document the types expected by the function but after that I would just use the objects as if they are the right type and let python throw an error if they aren't. I say this because type checking often gets in the way of duck typing and reduces the flexibility and generality of your code. For example, in python 2.x, isinstance(time_start, str) returns False for unicode strings (isinstance(time_start, basestring) is a better alternative).

The try/except is unnecessary as you are simply reraising every exception you catch.

Rather than trying to parse the strings yourself (which is quite error prone as AdHominem pointed out), you can use the datetime library instead.

If we apply these changes the result looks like:

from datetime import datetime

def time_difference(time_start, time_end):
    '''Calculate the difference between two times on the same date.

    Args:
        time_start: the time to use as a starting point
        time_end: the time to use as an end point

    Returns:
        the difference between time_start and time_end. For example:

        >>> time_difference('1500', '1600')
        60

        >>> time_difference('1100', '1310')
        130
    '''

    start = datetime.strptime(time_start, "%H%M")
    end = datetime.strptime(time_end, "%H%M")
    difference = end - start
    minutes = difference.total_seconds() / 60
    return int(minutes)
\$\endgroup\$
1
\$\begingroup\$

This code looks prone to errors when malformed strings are entered, especially since you are directly indexing the string at the end to process the time. You also have too many magic numbers in that code.

Conceptually, you should think about not using strings for time representation but instead use the time and datetime modules which are much more convenient for that purpose. Don't reinvent the wheel. You can just place assert statements right on the beginning of your function which check if the time parameters are valid and then just compare them.

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