Path: csiph.com!usenet.pasdenom.info!dedibox.gegeweb.org!gegeweb.eu!nntpfeed.proxad.net!proxad.net!feeder1-2.proxad.net!usenet-fr.net!nerim.net!novso.com!news2.euro.net!newsgate.cistron.nl!newsgate.news.xs4all.nl!post.news.xs4all.nl!not-for-mail
MIME-Version: 1.0
In-Reply-To: <CAE7MaQabqa2DT6bfiq3Ju1Kfyq2dMLFvebk3ynFbZnHxTSqsHA@mail.gmail.com>
References: <CAE7MaQabqa2DT6bfiq3Ju1Kfyq2dMLFvebk3ynFbZnHxTSqsHA@mail.gmail.com>
Date: Tue, 19 Mar 2013 01:16:19 +1100
Subject: Re: What are some other way to rewrite this if block?
From: Chris Angelico <rosuav@gmail.com>
To: python-list@python.org
Content-Type: text/plain; charset=ISO-8859-1
Precedence: list
Newsgroups: comp.lang.python
Message-ID: <mailman.3452.1363616201.2939.python-list@python.org>
Lines: 55
NNTP-Posting-Host: 2001:888:2000:d::a6
Xref: csiph.com comp.lang.python:41423

On Tue, Mar 19, 2013 at 12:56 AM, Santosh Kumar <sntshkmr60@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