WAKE UP! CodingBat alarm clock

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

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.

• 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. – Caridorc Aug 20 '16 at 13:50