[Python-ideas] Retrying EAFP without DRY

Mike Meyer mwm at mired.org
Wed Jan 25 01:52:01 CET 2012


On Wed, 25 Jan 2012 10:04:37 +1000
Nick Coghlan <ncoghlan at gmail.com> wrote:
> On Wed, Jan 25, 2012 at 4:45 AM, Mike Meyer <mwm at mired.org> wrote:
> > Which is not a code smell. However, if you can tell by reading the
> > code that it will only run once (or never run), like this one:
> >
> >     for i in range(1):
> >
> > Then it's a code smell!
> 
> I agree specifically in regards to range() with a literal argument,
> but it really depends on the iterator. Using break to force a single
> iteration can be nicer than calling next() and catching StopIteration.
> For example, code like the following makes it easy to special case the
> first entry in an iterator:
> 
>     walk_iter = os.walk(topdir)
>     for dirpath, files, subdirs in walk_iter:
>         # Special case the top level directory
>         break
>     else:
>        raise RuntimeError("No dir entry for {!r}".format(topdir))
>     for dirpath, files, subdirs in walk_iter:
>         # Process the subdirectories

And we've now gotten to the level of picking apart real examples. I
don't think a loop that never loops is a code smell because I think
break or continue are evil (the opposite, in fact; I'd like a
multi-level break/continue, but that's for another topic). I think
it's a code smell because there's a good chance it can be refactored
into straight-line code that would be better in a number of ways.

For instance, I would write your example like so:

    try:
        walk_iter = os.walk(topdir)
        dirpath, files, subdirs = next(wi)
        # Special case the top level directory
    except StopIteration:
        raise RuntimeError("No dir entry for {!r}".format(topdir))
    # other exception handling here, as appropriate.

    for dirpath, files, subdirs in walk_iter:
        # Process the subdirectories

Exactly what winds up in the try block will vary depending on
circumstances. Putting only the invocation of next in it would
duplicate your code. os.walk isn't documented as returning exceptions,
and ignores problems with listdir, so adding it seems to be a
nop. Handling the special case for the top level directory with this
try seems like a win, because all the exceptions from dealing with the
top level directory get grouped together. It might be useful to handle
exceptions from the subdir processing as well, but the choice is easy
to make with this version.

In any case, it's easier to change this version as needed to deal with
exceptions than the original version.

The other issue may be just me - I expect exiting a loop from the
bottom to be a normal flow path. So the for version reads to me like
raising RuntimeError is normal, not exceptional.

> Python has very powerful looping constructs already - we don't need
> more just because some folks have been trained to think that break and
> continue are evil and haven't considered whether or not these
> prejudices inherited from C and C++ are still relevant in Python.

Likewise, the try statement is very powerful. That break/continue may
or may not be evil is immaterial. The point is to make the try
statement more powerful.

So long as you incorrectly see this as "just another looping
construct", your conclusions will be flawed. Not necessarily wrong,
just flawed. It can also be used to fix LBYL and DRY code smells.

	<mike



More information about the Python-ideas mailing list