[Tutor] Review and criticism of python project

[Alan Gauld]

"GTXY20" <gtxy20 at gmail.com> wrote

> Sorry for the lengthly code response - all commenst are 
> appreciated - as
> mentioned I am quite new with Python - it is doing what I need it to 
> do but
> I think that it is a mess and needs to be cleaned up a little.

I've only skimmed it very quickly and a few things popped out at me.

1) Separate the UI from the logic. In particular dont put print 
statements
or raw_input statements in init methods - you will never be able to 
use
the class in a GUI context! Instead pass the values into the class
when you instantiate it.

2) If you must use import inside methods put it at the top - thats 
where
most readers will look for it. Its more conventional to put all 
imports in
a file at the top of the file.

3) Double underscore before a name in Python usually means
its a magic Python feature or a private attribute of a class. Using it 
for a
class name is very unusual. A single underscore would indicate the 
class
was "private" and not intended for general use and might be more
appropriate..

4) This section
>            self.uhdata={}
>            for line in lines:
>                uh, tf = line.split(',')
>                if uh in self.uhdata:
>                    f=self.uhdata[uh]
>                    if tf not in f:
>                        f.append(tf)
>                else:
>                    self.uhdata[uh]=[tf]
>
Looks like it should be cleaner. I don;t thionk you shouldneed all
those if/in checks using a dictionary. I can't be sure without 
spending
more time reading it than I've got right now but it looks wrong 
somehow.

5) What is the po8int of this:
>            for k,v in sorted(fcounts.items()):
>                Section=k
>                fqty=v
>                Sectionqtyoutfile.write ('%s\t%s\n' % (Section, 
> fqty))
>

Why not just
>            for k,v in sorted(fcounts.items()):
>                Sectionqtyoutfile.write ('%s\t%s\n' % (k,v))

The extra variables dont add anything and if you really prefer the
names then do this:
>            for Section,fqty in sorted(fcounts.items()):
>                Sectionqtyoutfile.write ('%s\t%s\n' % (Section,fqty))

> Thanks for any comments.

Thats as far as I got,

HTH,

-- 
Alan Gauld
Author of the Learn to Program web site
http://www.freenetpages.co.uk/hp/alan.gauld