slightly OT: Python BootCamp

[Steven D'Aprano]

On Thu, 03 Dec 2009 16:20:24 -0500, J wrote:

> The difference between me (learning a new language) and the guy teaching
> (who knows it inside and out)

I don't know about that. He's teaching some pretty atrocious habits. See 
below.


> His code:
[...]
> for (exten, list) in files.iteritems():
>     try:
>         f=open('extensions-%s.txt' % exten,'w')
>         f.write('\n'.join(list))
>     except:
>         assert 0, 'Unable to create output file extensions-%s.txt.' %
>         exten


Wrong! Bad! Don't do this!!!

There are no less than five errors here. Some people might argue that 
given the context (a short script), none of them matter in practice. For 
a script you use once then throw away, maybe. I've written sloppy scripts 
myself, and lazy can be a virtue. But *teaching* sloppy is a terrible 
thing -- even if you disagree that this are the sorts of errors that 
matter in this script, they are bad, bad habits that you should avoid.

Error #1: Bare excepts are nearly always wrong. Bare excepts catch 
*everything*, including exceptions caused by bugs in your code. Including 
the user trying to interrupt the code with ctrl-D. Don't use bare excepts 
unless you know what you're doing. Catch the specific exceptions you 
actually want instead.

Error #2: Wrapping multiple operations in a single try block. This 
conflates errors in four different operations:

generate the file name
open the file 
merge strings into a single newline-delimited string
write the strings to the file

The instructor assumes that only the open can fail. That's incorrect. His 
try...except will mask coding errors, as well as conflating open errors 
and write errors.

Error #3: Throwing away useful debugging information and replacing it 
with a generic error message that isn't useful.

If an error occurs, Python will generate a useful exception telling 
exactly what error occurred and why. If it happens in a file operation, 
it will include the error code. The given code throws that away and 
replaces it with a generic error that tells you nothing except that an 
assertion failed.

Error #4: Using assert for anything other than what it was designed for. 
If you want to break this script, run it with optimizations switched on 
and trigger a failure-mode (perhaps by removing write-permission from the 
current directory). With optimizations switched on, the assert statement 
will be compiled away, and the script will catch the open errors and 
ignore them.

Error #5: Failing to close each file after you're done with it. Python 
does close files for you, if you don't do it yourself, but when it does 
so is implementation dependent. Some versions of Python won't close them 
until the very end of the program, as it closes down. That means that if 
the list of files is large enough, you will run out of file descriptors 
and the program will stop working.


How would I re-write this? Just get rid of the try block and add a close:


for (exten, list) in files.iteritems():
    f=open('extensions-%s.txt' % exten,'w')
    f.write('\n'.join(list))
    f.close()

If an error occurs, Python's normal exception-raising mechanism will halt 
the script and print a useful error message.


If you want to mask the traceback from the user (but I'm not sure why you 
would want to...) then wrap the entire loop in a single try, catching 
keyboard interrupt separately:

try:
    for (exten, list) in files.iteritems():
        f=open('extensions-%s.txt' % exten,'w')
        f.write('\n'.join(list))
        f.close()
except KeyboardInterrupt:
    print "Interrupted by user."
    raise
except Exception:
    raise ValueError('failed for some unspecified reason')


That's still not quite "best practice" -- best practice would be to use a 
with block, but (to my shame) I'm not entirely familiar with that so I'll 
leave it for others.




-- 
Steven