Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Can you please tell me if there is a better way to do this?

people = {'123456':{'first': 'Bob', 'last':'Smith'},
          '2345343': {'first': 'Jim', 'last': 'Smith'}}

names = list()
first_names= set()
last_names = set()
for device in people:
    names.append(device)
    first_names.add(people[device]['first'])
    last_names.add(people[device]['last'])

I want to get a list of IDs, and sets of first and last Names. I know this code will work, but can I improve this?

share|improve this question

3 Answers 3

up vote 3 down vote accepted

The only possible improvement I see is to avoid unnecessary dictionary lookups by iterating over key-value pairs:

ids = list()
first_names = set()
last_names = set()

for person_id, details in people.items():
    ids.append(person_id)
    first_names.add(details['first'])
    last_names.add(details['last'])
share|improve this answer

Instead of appending the id's one by one, I would use:

ids = list(people.keys())

Note that I changed names to ids, as numbers don't seem like names to me. This is something that @jano's answer included but didn't explicitly state.

Then you can change your the rest of your code to:

first_names = set()
last_names = set()

for details in people.values():
     first_names.add(details['first'])
     last_names.add(details['last'])
share|improve this answer

I have been digging list comprehensions, think it makes this easy to read:

people = {'123456':{'first': 'Bob', 'last':'Smith'},
      '2345343': {'first': 'Jim', 'last': 'Smith'}}

names = people.keys()
first_names= set([people[name]['first'] for name in names])
last_names = set([people[name]['last'] for name in names])
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.