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