[CentralOH] Python Puzzle: Day generator

jep200404 at columbus.rr.com jep200404 at columbus.rr.com
Thu Feb 5 08:06:01 CET 2015


On Wed, 4 Feb 2015 20:55:14 -0500, Eric Floehr <eric at intellovations.com> wrote:

> Here is a group puzzle. I am interested in everyone's solutions.

> ... I need to iterate through days.
> So I thought to make a function that works like range() only with dates.

See http://colug.net/python/cohpy/20150205/
for what I did before looking at your code.

> And my solution (open to refactoring):
> https://gist.github.com/efloehr/3e1dd639f3c5c1b064b7

Here's some refactoring comments, and comparison to my code, 
particularly of my cell #5.

First, some easy PEP 8 stuff:

    Use isinstance() instead of type() == str
    Delete the spaces inside the [] of your comprehensions

I like day= in timedelta(day=1) in your code. It documents the value.
Likewise for days= at the bottom.

.strptime() in my code makes short work of parsing YYYY-MM-DD strings.
Your start_day is None is more correct than my not start_date.

Because of your range test, I see that your loop only goes once 
for bad step polarities, instead of the infinite loop I feared.
It would be good to tighten up the specification for what 
behavior is desired for whacko step polarity. Does the 
polarity handling in your day_generator() make your other 
code do nothing gracefully? 

The names range_start and range_end are misleading. 
Better names would be range_min and range_max.

Looking at your min()/max() lines, I wish there were a minmax() 
function that returned a tuple, kind of like divmod() does for 
division.

Can your while loop be simplified to the following?

    while range_min <= current_day <= range_max:

I like the way that testing against both the min and max, 
avoids the need to figure out which single test would suffice.

Doing from datetime import in your code made your other code 
easier to read than mine.

Even though my having d and start_date refer to the same object 
causes no bug, your current_day = copy(start_day) is more prudent 
than my d = start_date. Of course, I could just claim that d 
is a pronoun for start_date. :-)

Your day_step is a better argument name than my terse step, 
because yours documents the value better.

I'm a little surprised I did not take advantage of elif for 
my isinstance() code.

What else did I miss?


More information about the CentralOH mailing list