On Fri, 05 Dec 2008 13:27:35 +1300, Lawrence D'Oliveiro wrote:
> In message <[EMAIL PROTECTED]>, Cong
> Ma wrote:
>
>> The "if ... != None" is not necessary... "if PatchDatePat.search(f)"
>> is OK.
>
> I don't do that.
Perhaps you should?
Since the context has been deleted, it's hard to tell whether the code as
written by Lawrence is incorrect or merely bad style. Here's his original
piece of code, with the stupid formatting fixed to a more readable style:
for Entry in sorted(
f for f in os.listdir(PatchesDir) if PatchDatePat.search(f) != None
):
Patch = (open, gzip.GzipFile)[Entry.endswith(".gz")](
os.path.join(PatchesDir, Entry), "r")
... read from Patch ...
Patch.close()
Still pretty obfuscated. The for block could be made considerably more
readable. Here's one way:
opener = gzip.GzipFile if Entry.endswith(".gz") else open
Patch = opener(os.path.join(PatchesDir, Entry), "r")
... read from Patch ...
Patch.close()
Looking at the call to sorted(), Cong Ma suggested that
if PatchDatePat.search(f) != None
should be replaced by the much shorter:
if PatchDatePat.search(f)
The replacement is much more Pythonic style, but it could fail if
PatchDatePat.search() returns a false value (0, empty string, empty list,
etc) which is intended to count as a match. Without knowing what the
search method does, it is impossible to tell whether this is a risk or
not. On the other hand, Lawrence's code can also fail if the search
method returns an object which for some reason tests equal to None
despite being a match.
In other words, *both* techniques are fragile, because they make
assumptions about the sort of object the search method can return. The
best way of doing this test, given *only* the assumption that None
indicates no match, is with:
if PatchDatePat.search(f) is not None
This also happens to be (marginally) faster that either of the others, as
it relies only on an identity test, and doesn't need to make an equality
test or a __nonzero__ test, both of which require method lookups.
--
Steven
--
http://mail.python.org/mailman/listinfo/python-list
