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.

• Caridorc: Very fine answer, I would prefix boolean variables with is_ as it makes reading more intuitive and would nest ternaries to re-create the 4-state table that seems implied in the question (is_weekday x is_holyday) ignoring the repetition of 10:00 in the result to more closely follow the spec.