2010-12-17
A Python (non-)idiom that I should really avoid
One of my bad habits as a Python programmer is that I am both lazy and impatient. I will go out of my way to do something overly clever just because it saves me some boring boiler-plate code, and because Python is such a compact language already my threshold for this irritation is very low.
One of the things that this leads me to is writing conditional variable initializations like this:
var = None if condition: var = something()
I've recently come to the conclusion that this is a bad idiom that I
should avoid. On the 'good' side, I've saved a line and the annoyance
of writing the boiler-plate if/else (at the trivial cost of an extra
assignment). On the bad side, what I've found recently is that that
the 'var = None
' assignment can easily look a little bit too much
like it could have been a temporary addition done for debugging or code
testing. In fact I almost removed such an initialization today during a
cleanup pass of stripping my debugging and testing code from a program
before committing the changes to the repository. Actually putting the
else:
in makes it clear to me that both initializations are supposed
to be there.
(Of course there's all the general style and 'clever: -5 points' issues involved with writing code like this. But, well, I'm lazy and impatient and so I sometimes do this stuff anyways.)
I suppose I should be thankful that I don't really like Python's conditional expressions, and also that I'm still writing code for Python 2.4.6, which doesn't have them so I don't have a choice about liking or not liking them. Writing the above as:
var = something() if condition else None
is just far too clever, partly because I find that the order makes it hard to read.
Sometimes bugs have very small edit distances
One of the consequences of yesterday's SAN backend failure is that I found a bug in our spares handling code. It was the kind of bug where the program aborts, and when that happens we started getting email with Python exception backtraces that ended in what struck me as a very odd exception:
KeyError: False
Normally KeyErrors make a kind of sense, in that you see what
you're looking up and the question is either why you wound up using
that key or why that key isn't present when you thought it would
be. KeyError: False
was so far out of left field that it left me lost
until I finally stared at my code long enough to spot the problem.
It turns out that there is a world of difference between the following two (summarized) list comprehensions:
[... if x['vdev_state' == 'resilvering']]
[... if x['vdev_state'] == 'resilvering']
Yes indeed, looking up the results of a (constant) string equality
comparison is rather different from looking up the value of a key and
then comparing it with a string, even though the difference between the
two is only a repositioned ']
' (and the first doesn't look obviously
wrong when you're skimming). For a start, one works and the other
complains that you've never put False
in your property dict.
(The classical short edit distance mistake is of course C's '=
' for
'==
' error, which is not possible in Python.)
One awkward thing that this reveals is that I clearly never actually tested this code (whether with unit tests or just trying to make sure that it works). But that's another entry that reveals my ignorance of how to do TDD right.