On Wed, 2 Mar 2005, Luis N wrote:

> This code seems a little slow, is there anything in particular that
> jumps out as being not quite right.

Hi Luis,

Some comments:


You have an empty 'except' exception-handling block in the code:

######
        try:
            for i in files:
                page = os.path.join(write, i)
##      ... code cut
                    output.write(s)
        except:
            pass
######


Try not to do that.  *grin*


Having 'except: pass' is almost always a bad idea because it really makes
it hard to debug problems.  Make sure your exception handler does
something useful: as it is, the code can disguise all sorts of bugs
without giving any feedback to you.

It's not clear at all what the benefit of the custom exception handler is
in your code, so I'd recommend just dropping the try/except.



Another thing is that the code is a big long: it might be worthwhile to
break:

>                 page = os.path.join(write, i)
>                 f = open(os.path.join(pages, i), 'r')
>                 tmp = ""
>                 for line in f.readlines():
>                     line = line.rstrip()
>                     structure = open(os.path.join(publish, line))
>                     clean = structure.read()
>                     tmp = tmp + clean.rstrip()
>                     txt = textile(tmp) + '</body></html>'
>                     t = Template(txt)
>                     s = t.safe_substitute(title='Web-siter: %s' % i[:-5])
>                     output = open(page, 'w')
>                     output.write('')
>                     output.write(s)



off into its own helper function.  Let's pretend that we call this
function 'writePage()':

######
def writePage():   ## FIXME: the argument list here isn't quite right yet
    page = os.path.join(write, i)
    f = open(os.path.join(pages, i), 'r')
    tmp = ""
    for line in f.readlines():
        line = line.rstrip()
        structure = open(os.path.join(publish, line))
        clean = structure.read()
        tmp = tmp + clean.rstrip()
        txt = textile(tmp) + '</body></html>'
        t = Template(txt)
        s = t.safe_substitute(title='Web-siter: %s' % i[:-5])
        output = open(page, 'w')
        output.write('')
        output.write(s)
######

This breakup isn't quite right yet, and this needs to take in some
parameters.  So it needs a little work.  But this improves the code by
allowing you to concentrate on what it means to write a single page.



It also makes it easier to see a possible bug in the code: the last few
lines in the 'for' loop look suspicious:

######
        txt = textile(tmp) + '</body></html>'
        t = Template(txt)
        s = t.safe_substitute(title='Web-siter: %s' % i[:-5])
        output = open(page, 'w')
        output.write('')
        output.write(s)
######

It does look strange that the file is being open and rewritten over and
over, for each line in the file.  Are you sure you want to put that in the
body of the loop?


Best of wishes to you!

_______________________________________________
Tutor maillist  -  Tutor@python.org
http://mail.python.org/mailman/listinfo/tutor

Reply via email to