[Tutor] better way to write this code

eryksun eryksun at gmail.com
Fri Aug 24 14:18:19 CEST 2012


On Thu, Aug 23, 2012 at 2:33 PM, Norman Khine <norman at khine.net> wrote:
>
> import operator, json
> from BeautifulSoup import BeautifulSoup

If you have the source of TABLE_CONTENT, why don't you soup that
instead? Otherwise, nevermind.

> combos={0: 'id',
> 2: 'country',
> 3: 'type',
> 5: 'lat',
> 6: 'lon',
> 12: 'name' }

Here's the style I'd use for the above:

combos = {
  0: 'id',
  2: 'country',
  3: 'type',
  5: 'lat',
  6: 'lon',
  12: 'name',
}

Put each entry on its own line, indented by two spaces, and leave a
trailing comma on the last entry. The latter is especially important
in sequences of strings to prevent them from being
"silently"<=>"concatenated" if you were to add an entry and forget the
comma.

Below I've formatted the rest of your code to use 4 spaces instead of
tabs. Please follow PEP 8 unless local style rules take precedence. If
your code gets copied into a file that uses spaces, you end up with
the headache of mixing tabs and spaces. So try to use spaces since
that's what most people use. Your editor should be able to insert
spaces when you press <tab>.

> event_list = []
> for event in TABLE_CONTENT:
>     event_dict = {}
>     for index, item in enumerate(event):
>         if index == 8:
>             if item == '&nbsp;':
>                 event_dict['depth'] = '0'
>             else:
>                 event_dict['depth'] = item

You can simplify the above with a ternary expression. IMO, this
simplifies maintenance because you're not repeating the assignment to
"event_dict['depth']":

          if index == 8:
              event_dict['depth'] = item if item != '&nbsp;' else '0'

>         if index == 9:
>             try:
>                 items = item.split()
>                 if len(items) >= 2:
>                     event_dict['yield'] = items[-1]
>                 else:
>                     if item == '&nbsp;':
>                         event_dict['yield'] = '10'
>                     else:
>                         event_dict['yield'] = item
>             except:
>                 pass

I fail to see why you need a try/except here. Avoid using a bare
"except". Handle specific exceptions.

>         if index == 3:
>             if 'Atmospheric' in item:
>                 event_dict['fill'] = 'red'
>             if 'Underground' in item:
>                 event_dict['fill'] = 'green'

Should there be a default value for event_dict['fill']?

>     event_list.append(event_dict)
>     print event_dict
> event_list = sorted(event_list, key = operator.itemgetter('id'))

You may as well sort in place:

event_list.sort(operator.itemgetter('id'))

> f = open('detonations.json', 'w')
> f.write(json.dumps(event_list))
> f.close()
> print 'detonations.json, written!'

Use "with" to make sure the file gets closed even if there's an
exception. Also, you may as well "dump" directly to the file instead
of creating a string with "dumps":

with open('detonations.json', 'w') as f:
    json.dump(event_list, f)


More information about the Tutor mailing list