[Tutor] Zipfile and File manipulation questions.

Danny Yoo dyoo at hkn.eecs.berkeley.edu
Tue Oct 17 07:09:24 CEST 2006



On Mon, 16 Oct 2006, Chris Hengge wrote:

> Have you even read my code to see if you find it cryptic? I'm starting 
> to beleive people just read the one comment on possibly using better 
> naming conventions and assumed I had picked completely irrelivent names.

Hi Chris,

Sometimes one of us (or a few of us) might rush out our answers a bit too 
quickly.  I think we're just trying to make good on that "24 hours or your 
answer is free" thing.  *wink*


The last version I've seen of your code is:

     http://mail.python.org/pipermail/tutor/2006-October/050059.html

The comment I'd make on extractZip is that it is a bit nested and long. 
You might want to apply a refactoring to flatten things out a bit. 
Concretely, if we look at extractZip():

###########################################################
def extractZip(filename):
     ...
     zFile = zipfile.ZipFile(filePathName.strip('"'), "r")
     for aFile in zFile.namelist():
         for ext in ['.cap', '.hex', '.fru', '.cfg', '.sdr']:
              if aFile.lower().endswith(ext):
                  ...
############################################################

we see that it has several levels of logic.  But it can have its nesting 
reduced if we provide a helper function to recognize interesting 
filenames:

########################################################
def isInterestingFile(fileName):
     """Returns True if the fileName ends with a needed
     extension, and False otherwise."""
     for ext in ['.cap', '.hex', '.fru', '.cfg', '.sdr']:
         if fileName.lower().endswith(ext):
             return True
     return False
########################################################



If we have this isInterestingFile() as a helper function, the main logic 
in extractZip is easier to see:

#########################################################
def extractZip(filename):
     ...
     zFile = zipfile.ZipFile(filePathName.strip('"'), "r")
     for aFile in zFile.namelist():
         if isInterestingFile(aFile):
             ...
#########################################################

Since the helper function is standalone, it's just physically easier to 
see that what's distinguishing interesting files from non-interesting ones 
is its treatment of the extension, and I hope you agree that the structure 
of extractZip() improves a bit.


You may want to consider restructuring your program like this, using 
things like helper functions, so that commenting every single line of code 
becomes less necessary to understand the program.

As it stands, every line of code is commented --- this is usually 
overkill.  If you find yourself needing to do it to understand the 
program, consider the possiblity that those comments might be acting as a 
crutch.  The approach that most of us here recommend is to simplify the 
program structure so that it's easy to understand.

Comments are necessary, but they should be used judiciously.  See how far 
you can get by using helper functions to break out blocks of interesting 
and useful code.


If you want another example of a possible refactoring, feel free to ask 
the group.  Good luck!


More information about the Tutor mailing list