Question about None

Steven D'Aprano steve at REMOVETHIS.cybersource.com.au
Sat Jun 13 02:34:00 EDT 2009


Jean-Michel Pichavant wrote:

> 
>>   def setNext(nxt):
>>     assert nxt==None or isinstance(nxt, Node), "next must be a Node"
>>     self.next = nxt
>>
>> works ok, but it's uglier than it ought to be.
>>
>>   
> I don't find it that ugly. It's accurate, easy to read and understand.

Actually, it's inaccurate. There are *three* bugs in that code, two in the
same line. (Admittedly, one of the three is closer to a nitpick than a
bug.) You already allude to one of them:


> "What else ?" would say a famous coffee representative.
> If you like perfection, replace nxt==None by nxt is None.

Delete the "if you like perfection" part. Testing for equality against None
in this case is simply *wrong*. Arbitrary objects can be made to test equal
to None -- for instance, the Null object pattern. Unless you wish to allow
such "null" objects as well as None, then "nxt == None" does not do what
you want to do: it fails to reject some things that should be rejected.

The second bug is that the line uses assert for runtime value/type testing.
That's bad, because asserts are disabled when you run Python with the
optimisation flag. That means that your program will stop performing the
necessary test.

assert is for testing program logic[1], not argument testing. If you're ever
tempted to write an if...elif...else block that includes the comment, "this
should never happen, but just in case" then you have a good candidate for
an assert. A good use for assert is for checking that your program logic is
correct, not for testing user-arguments:

def process(text):
    for word in text.split():
        word = somefunction(word)
        # say that it's important that word contains no newlines
        # you're sure that somefunction() *won't* introduce any,
        # but you want to be positive
        assert "\n" not in word
        do_something_with(word)
    
In this case, assert is appropriate: the code *should* work if the assert is
disabled, but it will give you some protection from bugs in your code.
Whenever you're tempted to use assert, ask yourself "if I deleted this test
from my code, would the program still run correctly?". If the answer
is "possibly not", then don't use an assert.


The third bug (the nitpick) is a bug in the API: you name the argument "nxt"
with no e, but in the error message, you call it "next" with an e. The fact
that any intelligent person should be able to guess what parameter the
error is referring to doesn't make it less wrong.

So, fixing the three issues:

def setNext(next):
    if not (next is None or isinstance(next, Node)):
        raise ValueError("next must be a Node")
    self.next = next




[1] Arguably, it's also good for poor-man's unit testing when you're too
lazy to write proper tests.



-- 
Steven




More information about the Python-list mailing list