What are some other way to rewrite this if block?

Chris Angelico rosuav at gmail.com
Mon Mar 18 15:16:19 CET 2013


On Tue, Mar 19, 2013 at 12:56 AM, Santosh Kumar <sntshkmr60 at gmail.com> wrote:
> This simple script is about a public transport, here is the code:
>
> def report_status(should_be_on, came_on):
>     if should_be_on < 0.0 or should_be_on > 24.0 or came_on < 0.0 or
> came_on > 24.0:
>         return 'time not in range'
>     elif should_be_on == came_on:
>         return 'on time'
>     elif should_be_on > came_on:
>         return 'early'
>     elif should_be_on < came_on:
>         return 'delayed'
>     else:
>         return 'something might be wrong'
>
> print(report_status(123, 12.0))

Well, for a start, I wouldn't compare for equality there. What's your
definition of "on time"? In Melbourne, for instance, a service is
deemed "on time" if it's less than 1 minute early and less than 5
minutes late. This is probably too broad, but I would guess that up to
1 minute (or at least half a minute) either side should be considered
on time.

Are you catering here for custom objects or NaNs that might not be
equal, less, or greater? If not, drop the else and just have three
branches - for instance:

if should_be_on >= came_on + 0.5: # Up to half a minute early is okay
    return 'early'
elif should_be_on <= came_on - 1.0: # Up to one minute late is okay
    return 'delayed'
else:
    return 'on time'

As a matter of readability, incidentally, I'd be inclined to invert
the conditions and check the time of arrival against the expected
time:

if came_on < should_be_on - 0.5: # Up to half a minute early is okay
    return 'early'
elif came_on > should_be_on + 1.0: # Up to one minute late is okay
    return 'delayed'
else:
    return 'on time'

I don't understand your bounds check, though. Are you working with
floating point hours in the day? (If so, it's still not necessarily
right - it's not uncommon to refer to the hours post-midnight as
24:00, 25:00, etc. But for a toy and a PoC, that would work.) But you
then pass the integer 123 as the first arg, which will fail that
check. What _is_ your data type?

ChrisA



More information about the Python-list mailing list