WAKE UP! CodingBat alarm clock

Question

I'm brand new at Python, but I've been working in general at granulating my functions and being as self-readable as possible without suffering readability. This CodingBat question is a little interesting because it demands a function with a lot of repetitive case checks. I decided, rather than having a bunch of nested ifs, it might be better to break them out into separate functions, but I'm not sure if this actually goes -against- my goal of better readability. However, I decided to break out is_weekday because the number encoding wasn't immediately clear. I also wasn't sure if it'd be better to have alarm_time(is_weekday(day), '10:00', 'off') or do alarm_time(day, '10:00', 'off') and have alarm_time handle the is_weekday(day) check. I decided the first way because it seemed there was a separation of concern issue otherwise, but I'm not sure.

I was at first hesitant to ask because this might be gray-space into asking a general opinion, but I'm specifically asking in terms of this exercise to use in similar situations.

Given a day of the week encoded as 0=Sun, 1=Mon, 2=Tue, ...6=Sat, and a boolean indicating if we are on vacation, return a string of the form "7:00" indicating when the alarm clock should ring. Weekdays, the alarm should be "7:00" and on the weekend it should be "10:00". Unless we are on vacation -- then on weekdays it should be "10:00" and weekends it should be "off".

def is_weekday(day):
    return 1 <= day <= 5

def alarm_time(is_weekday, weekday_time, weekend_time):
  if is_weekday:
    return weekday_time
  return weekend_time

def alarm_clock(day, vacation):
  if vacation:
    return alarm_time(is_weekday(day), '10:00', 'off')
  return alarm_time(is_weekday(day), '7:00', '10:00')


def test_fun(fun):
    cases = [[1, False, '7:00'], [5, False, '7:00'], [0, False, '10:00'], [6, False, '10:00'], [0, True, 'off'], [6, True, 'off'], [1, True, '10:00'], [3, True, '10:00'], [5, True, '10:00']]

    print('{:^30}     {:^11}'.format('Expected', 'Run'))
    for case in cases:
        funstr   = '{}({}, {})'.format(fun.__name__, case[0], case[1])
        result   = fun(case[0], case[1])
        expected = case[-1]
        success  = result == expected

        print('{:21} -> {:5} ==> {:5}  {}'.format(funstr, expected, result, success))

The test cases were positive:

>>> test_fun(alarm_clock)
           Expected                    Run    
alarm_clock(1, False) -> 7:00  ==> 7:00   True
alarm_clock(5, False) -> 7:00  ==> 7:00   True
alarm_clock(0, False) -> 10:00 ==> 10:00  True
alarm_clock(6, False) -> 10:00 ==> 10:00  True
alarm_clock(0, True)  -> off   ==> off    True
alarm_clock(6, True)  -> off   ==> off    True
alarm_clock(1, True)  -> 10:00 ==> 10:00  True
alarm_clock(3, True)  -> 10:00 ==> 10:00  True
alarm_clock(5, True)  -> 10:00 ==> 10:00  True

Answer 1

Testing

I find it weird that alarm_clock is passed to test_fun as a parameter, but the cases are hard-coded within test_fun. They should either both be hard-coded, or both passed as parameters.

Stylistically, cases would be better as a list of tuples. Tuples have a connotation of being non-homogeneous collections of uniform length (like a single row in a database table). Lists have a connotation of being homogeneous collections of undetermined length.

The way you split case[0], case[1], and case[-1] is awkward. I would put the singular expected result first, then use tuple unpacking.

For clarity, you should print repr(param) instead of param.

Here's how I would write it:

def test_fun(fun, cases):
    print('{:^30}     {:^11}'.format('Expected', 'Run'))
    for expected, *params in cases:
        funstr = '{}({})'.format(fun.__name__, ', '.join(repr(p) for p in params))
        result = fun(*params)
        success = result == expected
        print('{:21} -> {:5} ==> {:5}  {}'.format(funstr, expected, result, success))

test_fun(alarm_clock, [
  ('7:00', 1, False),
  ('7:00', 5, False),
  ('10:00', 0, False),
  …
])

Better yet, consider dropping test_fun altogether and using a doctest instead.

Implementation

For a simple problem like this, there are many ways to write it, and choosing the "best" one is a matter of taste. Personally, I would prefer to keep it simple so that you don't have to jump back and forth among three functions to understand the code.

def alarm_clock(day, vacation):
    """
    Given a day of the week encoded as 0=Sun, 1=Mon, 2=Tue, ...6=Sat, and a
    boolean indicating if we are on vacation, return a string of the form
    "7:00" indicating when the alarm clock should ring. Weekdays, the alarm
    should be "7:00" and on the weekend it should be "10:00". Unless we are on
    vacation -- then on weekdays it should be "10:00" and weekends it should
    be "off".

    >>> alarm_clock(1, False)
    '7:00'
    >>> alarm_clock(5, False)
    '7:00'
    >>> alarm_clock(0, False)
    '10:00'
    """
    weekday = 1 <= day <= 5
    return ( '7:00' if weekday and not vacation else
            '10:00' if weekday ==      vacation else
            'off')

The CodingBat editor seems to encourage two spaces per level of indentation. PEP 8, the official Python style guide, says that you should use four spaces.