Programming Idiomatic Code

[Steve Lianoglou]

I don't know about idiomatic, but here's a quick list of somethings
I'd change.

1) In general, I don't think it's a good idea for a called function to
blow out the system on error. So, in this example, I wouldn't have
raised errors be caught in the same function only to call os.exit. I'd
either deal with the exception in some sane way, or have it bubble up
to the caller to have it then decide what it should do in the face of
the error.


> def read_id_file(filename):
>     """ reads a file and generates a list of ids from it"""
>     ids = [ ]
>     try:
>         id_file = open(filename, "r")
>         for l in id_file:
>             ids.append( l.strip("\n") )
>         id_file.close()
>     except e:
>         print e
>         os._exit(99)
>     return ids

Maybe use a list comprehension here (try/except clauses excluded in
reference to previous comment @ top)

def read_id_file(filename):
    id_file = open(filename, 'r')
    ids = [line.strip() for line in id_file.xreadlines()]
    id_file.close()
    return ids

If you're shooting for only Python 2.5, you can use ``with`` (see
http://docs.python.org/tut/node10.html#cleanup-with) to make that even
more concise.


> def generate_count(id_list):
>     """ takes a list of ids and returns a dictionary of cities with
> associated counts"""
>     city_count = {}
>     for i in id_list:
>         url = "%s%s" %(base_url,i)
>         req = urllib2.Request(url)
>         try:
>             xml = parse(urllib2.urlopen(url)).getroot()
>             city  = xml.findtext('user/city')
>         except e:
>             print e.reason
>             os._exit(99)
>         try:
>             city_count[city] += 1
>         except:
>             city_count[city] = 1
>     return city_count

I maybe wouldn't bail on the error raised when the ``parse`` function
is called ... I'd rather skip the error and try the next one (but
that's just my preference, not sure if it's good/bad style)

Also, instead of doing this:

> try:
>    city_count[city] += 1
> except:
>    city_count[city] = 1

I like using the dict.get() methods ... which would make that a one
liner:

city_count[city] = city_count.get(city, 0) + 1

Now I gotta get back to work, too ... good luck getting that job! ;-)

-steve