HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

WAKE UP! CodingBat alarm clock

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
wakealarmcodingbatclock

Problem

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  {: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 ==>

Solution

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.

Code Snippets

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),
  …
])
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')

Context

StackExchange Code Review Q#139185, answer score: 5

Revisions (0)

No revisions yet.