I have a looping Python code with the following nested structure:

• account is a randomly chosen dictionary that sometimes contains Campaigns key.
  • Campaigns key is a list of dictionaries that sometime contain Audiences key.
    • Audiences key is list of dictionaries.

Situation:

• My code selects a random campaign from Campaigns -> then a random Audience from that Campaign's Audiences.
• Sometimes Campaigns and Audiences keys don't exist or are empty; so before making a random selection, I do a quick IF statement.
• If the list is empty or doesn't exist, I remove the dictionary from its parent list.

Question:

I was wondering if the following code can be written in a shorter / cleaner way:

if len(account.get("Campaigns", [])) > 0:
    campaign = random.choice(account.get("Campaigns", [{}]))

    if len(campaign.get("Audiences", [])) > 0:
        audience = random.choice(campaign.get("Audiences", [{}]))

    else:
        account["Campaigns"].remove(campaign)
        continue
else:
    accounts.remove(account)
    continue

CodePudding user response：

Not sure is it your requirement, but do you need to remove that empty lists for Campaigns or Audiences? It's easier for check and adding future stuff when you store empty list/object under the key.

But the way I would write this code is:

if (campaigns := account.get("Campaigns")) :
    campaign = random.choice(campaigns)

    if (audiences := campaign.get("Audiences")) :
        audience = random.choice(audiences)
    else:
        account["Campaigns"].remove(campaign)
        continue
else:
    accounts.remove(account)
    continue

It uses python 3.8 feature of Assignment expressions

CodePudding user response：

The only problem with your code is that it violates the DRY principle. What would you do with a third our fourth layer of keys? I think this would be more canonical Python.

import random
from typing import Dict, Optional

def get_random_value_item(
    dict_: Dict[str, List[Dict]], *keys: str
) -> Optional[Dict]:
    """
    Recursively, randomly choose from dict_[keys[0]] values.

    :param dict_: Dictionary of lists of dictionaries ...
    :param keys: One key per depth of dictionary
    :returns: random.choice(random.choice(dict_[keys[1]]), dict_keys[0]) ...

    If a list dict_[keys[n]] is empty, remove it and return None
    If dict_[keys[n]] is absent, return None
    """
    if not keys:
        return dict_
    try:
        next_dict = random.choice(dict_[keys[0]])
        return get_random_value_item(next_dict, keys[1:])
    except KeyError:  # key was not found, get failed
        pass
    except IndexError:  # key found but list was empty
        dict_.pop(keys[0])
    return None


# one short test
account: Dict[str, List[Dict]] = {
    "Campaigns": [{"Audiences": [{1: 2}, {3: 4}, {5: 6}]}],
}

print(get_random_value_item(account, "Campaigns", "Audiences"))
print(account)

CodePudding user response：

You could avoid re-reading the dictionary keys and use Truthy values instead of len(). This would make the code less cluttered but not necessarily much faster:

campaigns = account.get("Campaigns")
if campaigns:
    campaign  = random.choice(campaigns)
    audience = random.choice(campaign.get("Audiences") or [None])
    if not audience:
       campaigns.remove(campaign)
       continue
else:        
    if accounts.remove(account)
    continue

You could also reduce the levels of indentation by processing campaigns and audiences sequentially (given that you issue a continue in cases where there is no data:

campaign = random.choice(account.get("Campaigns") or [None])
if not campaign:
    accounts.remove(account)
    continue  
audience = random.choice(campaign.get("Audiences") or [None])
if not audience:
    account["Campaigns"].remove(campaign)
    continue