On Wed, 25 Jan 2012 10:04:37 +1000
Nick Coghlan
On Wed, Jan 25, 2012 at 4:45 AM, Mike Meyer
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.