[Tutor] Review and criticism of python project

[GTXY20]

thanks for the feedback - i will go through your comments line by line
adjust and test and will re-post when complete.

GTXY20

On Jan 4, 2008 9:29 PM, Kent Johnson <kent37 at tds.net> wrote:

> Tiger12506 wrote:
>
> > Ouch. Usually in OOP, one never puts any user interaction into a class.
>
> That seems a bit strongly put to me. Generally user interaction should
> be separated from functional classes but you might have a class to help
> with command line interaction (e.g. the cmd module in the std lib) and
> GUI frameworks are usually built with classes.
>
>
> >>                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]
> >
> > This can read
> >
> > try:
> >   f = self.uhdata[uh]
> > except KeyError:
> >   self.uhdata[uh] = []
> > finally:
> >   self.uhdata[uh].append(tf)
>
> These are not equivalent - the original code avoids duplicates in
> self.uhdata[uh].
>
> Possibly self.uhdata[uh] could be a set instead of a list. Then this
> could be written very nicely using defaultdict:
>
> from collections import defaultdict
>   ...
>   self.uhdata = defaultdict(set)
>   ...
>   self.uhdata[uh].add(tf)
>
> >>            for uh, Sections in self.uhdata.items():
> >>                Sections.sort()
> >
> > This will not work. In the documentation, it says that dictionary object
> > return a *copy* of the (key,value) pairs.
> > You are sorting those *copies*.
>
> No, the original code is fine. The docs say that dict.items() returns "a
> copy of a's list of (key, value) pairs". This is a little misleading,
> perhaps. A dict does not actually contain a list of key, value pairs, it
> is implemented with a hash table. dict.items() returns a new list
> containing the key, value pairs.
>
> But the keys and values in the new list are references to the same keys
> and values in the dict. So mutating the values in the returned list, via
> sort(), will also mutate the values in the dict.
>
>
> >>            missingpgcounts={}
> >>            fmissingpgcounts=[]
> >>            for x in self.uhdata:
> >>                for f in self.uhdata[x]:
> >>                    if f not in fmissingpgcounts:
> >>                        fmissingpgcounts.append(f)
> >
> > fmissingpgcounts = [f for f in self.uhdata.itervalues() if f not in
> > fmissingpgcounts]
> > Ahhh... This looks like a set.
> > fmissingpgcounts = set(self.uhdata.itervalues())
>
> Again, this is not quite the same thing. The original code builds a set
> of the contents of the values. You are building a set from the values
> (which are already lists). You still need one loop:
> fmissingpgcounts = set()
> for v in self.uhdate.itervalues():
>   fmissingpgcounts.update(v)
>
> > self.pgcounts = dict((x,0) for x in fmissingpgcounts)
>
> or, self.pgcounts = dict.fromkeys(fmissingpgcounts, 0)
>
> That's all I have energy for...
> Kent
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.python.org/pipermail/tutor/attachments/20080104/b1f8d923/attachment-0003.htm