Don't you just love writing this sort of thing :)

[Steven D'Aprano]

On Fri, 05 Dec 2008 13:27:35 +1300, Lawrence D'Oliveiro wrote:

> In message <mailman.4920.1228383577.3487.python-list at python.org>, 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