In <11*********************@m79g2000cwm.googlegroups. com>, Ritesh Raj
Sarraf wrote:
I have some remarks on exception handling.
1) compress_the_file() - This function takes files as an argument to it
and put all of them into a zip archive
def compress_the_file(zip_file_name, files_to_compress, sSourceDir):
'''Condenses all the files into one single file for easy
transfer'''
try:
import zipfile
except ImportError:
sys.stderr.write("Aieeee! module not found.\n")
`zipfile` is part of the standard library. So it should be there. If it
isn't then you write the message and the program will run into a name
error exception because `zipfile` isn't imported then but you use it later
on.
try:
os.chdir(sSourceDir)
except:
#TODO: Handle this exception
pass
Don't do that! This catches every exception and ignores it. Write just a
comment instead that says something like `TODO: add proper exception
handling`.
try:
filename = zipfile.ZipFile(zip_file_name, "a")
except IOError:
#INFO By design zipfile throws an IOError exception when you
open
# in "append" mode and the file is not present.
filename = zipfile.ZipFile(zip_file_name, "w")
except:
#TODO Handle the exception
sys.stderr.write("\nAieee! Some error exception in creating zip
file %s\n" % (zip_file_name))
sys.exit(1)
filename.write(files_to_compress, files_to_compress,
zipfile.ZIP_DEFLATED)
filename.close()
Poor choice of names IMHO. I expect `filename` to be a file name, i.e. a
string and `files_to_compress` is plural so one may expect a list or
another iterable.
This `import` wrapped into `try`/`except ImportError` idiom sprinkled all
over the source is a bit odd. Just do your imports and wrap the main
programs function into `try`/`except` if you want to catch the
`ImportError`\s. If you stop the program anyway this leads to much less
cluttered source code.
And it maybe would make sense to print the actual catched `IOError`\s too
because it may be a valueable information for the user *why* it failed.
For example because he has no rights or the file system is full.
Ciao,
Marc 'BlackJack' Rintsch