Path: csiph.com!fu-berlin.de!uni-berlin.de!individual.net!not-for-mail
From: Gregory Ewing <greg.ewing@canterbury.ac.nz>
Newsgroups: comp.lang.python
Subject: Re: Simple exercise
Date: Fri, 11 Mar 2016 12:24:08 +1300
Lines: 28
Message-ID: <dkee0qF5s1dU1@mid.individual.net>
References: <mailman.117.1457600573.15725.python-list@python.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
X-Trace: individual.net gW2C3LFgAIZ/aTASUqYXpAvohxkqr7aETbsULgYrA6a1jn/h2H
Cancel-Lock: sha1:pF0JpqFL+3vk1vJNGDK2em49pY0=
User-Agent: Mozilla Thunderbird 1.0.5 (Macintosh/20050711)
X-Accept-Language: en-us, en
In-Reply-To: <mailman.117.1457600573.15725.python-list@python.org>
Xref: csiph.com comp.lang.python:104560

Rodrick Brown wrote:
>       if m.group(1) not in od.keys():
>         od[m.group(1)] = int(m.group(2))
>       else:
>         od[m.group(1)] += int(od.get(m.group(1),0))

Others have pointed out what's wrong with this, but here's
a general tip: Don't repeat complicated subexpressions
such as m.group(1). Doing so makes the code hard to read
and therefore hard to spot errors in (and less efficient
as well, although that's a secondary consideration).

Instead, pull them out and give them meaningful names.
Doing so with the above code gives:

   name = m.group(1)
   value = m.group(2)
   if name not in od.keys():
     od[name] = int(value)
   else:
     od[name] += int(od.get(name, 0))

Now it's a lot eaier to see that you haven't used the
value anywhere in the second case, which should alert
you that something isn't right.

-- 
Greg