[Tutor] Review and criticism of python project

Tiger12506 keridee at jayco.net
Mon Jan 7 23:33:53 CET 2008

> 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.

Sorry... made that a little too general. I just have personal experiences
with putting a whole bunch of raw_inputs in a class once and learned never
to make that mistake again when I wanted to use the code for something else.

>>>                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.

Ah hah! Yes knew about hashes... It was the references that got me. I need
to learn to use id() at the python prompt a little more often. ;-)

>>>            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

Hmm... Apparently I didn't have enough energy. I must remember to eat
breakfast more often, it's affecting how I think... You're completely right
of course, Kent! What would all of us do without you?

More information about the Tutor mailing list