[Tutor] Should I use generators here?

Alan Gauld alan.gauld at freenet.co.uk
Mon May 8 12:43:33 CEST 2006


Hi Tony,

One point that jumps out at me is:

# ------------------
def ProcessFiletype(Thesefiles, Summary, Stats):
# ...
        try:
            fh=open(Onefile, "r")
        except IOError:
            print("\nFATAL ERROR ocurred opening %s for input" % 
Onefile)
        else:
            try:
                Filecontents = fh.readlines()  # these files are very 
small,
# ---------------

The whole idea of try/except style is that you can write
the happy path code in one place and put all the exception
handling at the bottom. This helps to clarify the code significantly.
Thus you should probably do:

        try:
            fh=open(Onefile, "r")
            Filecontents = fh.readlines()  # these files are very 
small,

            # other code here
            Summary[Onefile] = deepcopy(Stats)    # associate each

            fh.close()
        except IOError:
            print("\nFATAL ERROR ocurred opening %s for input" % 
Onefile)
        except IOError:
                print("\nFatal error occurred reading from %s\n\n" % 
InputFilename)

The problem of course is that you have two cases with the same 
exception
but you can handle that within the except by looking at the exception
information (or more simply by setting a flag to ok once the file is 
opened
which is initially simpler but that can lead to its own problems 
maintaining
the flag status!.)

Even if you decide to stick with two try blocks you shouldn't need the
else construct. Either break or exit as part of the first except 
handler.
The fewer levels of control you have in the program the easier it is 
to
see what is happening and the lower the likeliehood of indentation
level bugs being introduced.

###############################
def ProcessAllFiles(Summary, Stats, FileTypes, FiletypeStats):

    """Iterates over all Files in current directory that have the 
extensions
in Filetypes"""

    from glob import glob
    LongestFilenameLen = 0
    for Filetype in FileTypes:
        TheseFiles = glob("*" + Filetype)
        TheseFiles.sort()
        FiletypeStats[Filetype]=len(TheseFiles)
        Longest = ProcessFiletype(TheseFiles, Summary, Stats)
        if( Longest > LongestFilenameLen):
            LongestFilenameLen = Longest

    return LongestFilenameLen

#############################

I'm also somewhat confused as to why you seem so interested
in the length of the longest filename? You seem to spend a fair
bit of time finding the longest filename length?

No big performance improvements there however I'm just
looking at the superficial level of the code first.

HTH,

Alan G 




More information about the Tutor mailing list