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 -- http://mail.python.org/mailman/listinfo/python-list