1

I got the following function written in python.

def get_campaign_id(campaigns, campaign_name):
  for c in campaigns:
    if c['name'] == campaign_name:
      return c['id']
  return None

campaigns is a list of dicts, e.g. 

campaigns = [{'name': 'campaign1', 'id': 91203}]

and campaign_name is a str

georgez
  • 737
  • 1
  • 8
  • 20
  • 7
    This should be posted on http://codereview.stackexchange.com/. – Christian Berendt Jul 25 '14 at 17:01
  • 1
    possible duplicate: http://stackoverflow.com/questions/6981717/pythonic-way-to-combine-for-loop-and-if-statement – Sid Jul 25 '14 at 17:02
  • could be done in one line: `return c['id'] for c in campaigns if c['name'] == campaign_name else None` – agconti Jul 25 '14 at 17:04
  • 1
    There is absolutely nothing un-pythonic about your method. It is explicit, clear and easy to understand. Remember that "one liner" doesn't mean "pythonic". – Burhan Khalid Jul 25 '14 at 17:05
  • @agconti That returns an iterator, not the id of the first matching dictionary. – chepner Jul 25 '14 at 17:07
  • @BurhanKhalid meh .. looping through the whole list to find one id over and over again is not very efficient ... (especially if you have to do it multiple times) – Joran Beasley Jul 25 '14 at 17:09
  • Perhaps the only improvement you can do is sort the list by the campaign names, but other than that I think its great. @JoranBeasley given the constraints of the system (list of dicts), there is very little that can be done other than that. Your answer that creates another structure may not be possible if his method is being called by another function (so he cannot create it "once"). – Burhan Khalid Jul 25 '14 at 17:12
  • 1
    meh a list is not the proper data structure for this ... – Joran Beasley Jul 25 '14 at 17:18

1 Answers1

1
mapping =dict([c["name"],c["id"]) for c in campaigns]) #save this ... dont recreate it all the time
print mapping[campaign_name] # get the id by campaign name

is probably a better way to do it ...

Joran Beasley
  • 110,522
  • 12
  • 160
  • 179
  • Yes... but it uses O(n) memory instead of O(1). – Bakuriu Jul 25 '14 at 17:20
  • what makes you think a list is O(1) memory useage? ... Im pretty sure most of the time the whole list is in memory ... – Joran Beasley Jul 25 '14 at 17:23
  • When you speak of memory asymptotic complexity you generally include only *extra* memory used. In other words: your solution doubles the amount of memory required. `dict` uses quite a bit more memory than a list of the same size with the same contents, so your code uses strictly more than twice the memory of the original. – Bakuriu Jul 25 '14 at 17:29
  • I doubt it ... since its a list of dicts then that implies it would use `dict memory * list size` I would be curious to see an analysis though (tbh Im not sure how to examine memory footprint) (Im also assuming you could discard the original list) – Joran Beasley Jul 25 '14 at 17:31
  • Where does the multiplication comes from? My O(n) refers to the fact that the `dict` pointed to by `mapping` has a size that is proportional to the length of the original list (`campaigns`). Even if you can disregard the original list, in order to built the dict they must "live together" for some time. The original solution doesn't even allocate any new object. – Bakuriu Jul 25 '14 at 17:47
  • I see... yes if both objects are living together in the program space the it is 2x as much memory (although thats likely not an issue with modern hardware) I was referring to the original list being `[dict1,dict2,dict3,...]` thus size is `[overhead_of_dict+content,overhead_of_dict+content,...]` where as my solution only has one overhead of `dict + content` (ie the mapping datastructure is likely a smaller memory footprint than the campaigns list datastructure), I thought you were saying mapping would have a larger overhead than the list – Joran Beasley Jul 25 '14 at 18:15