Random assignment function with caveat

Assign equal number chores to each person (assertion made elsewhere to ensure chores % number of people == 0) with the stipulation that each chore must not be one which the person was assigned the previous time.

My simpleton solution (there has to be a cleverer way):

```
import random

chores = ["hoovering", "dusting", "wash dishes", "tidy garden",
"clean windows", "empty dishwasher", "dust", "cook"]

people_data = {"David": {"email": "example@gmail.com",
"chores" : []},
"Mark": {"email": "example@gmail.com",
"chores": []},
"Anaya": {"email": "example@gmail.com",
"chores": []},
"Cooper": {"email": "example@gmail.com",
"chores": []}
}

prev_assignment_data = None

def assign_chores(chore_list, prev_assignment_data, people_data):
"""Returns a dict with assign chores to each person

Arguments:
chore_list -- a list of the chores to be assigned
prev_assignment_data -- if there has been prev chore assignments this
will be a dict containing said assignments.
Otherwise, it will have no value ('None')
people_data -- a dict of the names and the email address
associated with those names. Assigned chores
will be appended to this dict and dict
returned.
"""

# if no previous data available, assign each a random chore
if not prev_assignment_data:
while chore_list:
for name in people_data.keys():
chore = random.choice(chore_list)
chore_list.remove(chore)
people_data[name]["chores"].append(chore)

# if prev data available, assign each chores not the same as previous
else:
new_chores = False
while not new_chores:

Well, you have your code in a function, but it is still top down. It makes it harder to read, and much harder to improve.

If you instead break the code down into separate functions you'll have an easier time improving code. You'll also not repeat your self.

Make use of annotations!

def assign_chores(chore: list, prev_assignment: dict or None, persons: dict):
    ...

that way you don't have to litter your variable names with clues to what types they might be.

I think your trying to create a deep copy and not a shallow copy:

chore_list_copy = chore_list

to make a deep copy;

chore_list_copy = chore_list[:]

that way you'll actually get a copy.

for name in people_data.keys():

is the same as

for name in people_data:

Instead of working with the objects in people_data i suggest you work with data structures. It becomes easier to manipulate that way.

The data you care about can be represented with:

chores_dist = [[] for _ in range(len(persons))]

and if previous is summited:

prev_chores = [person["chores"] for person in prev_assigment.values()]

Work with data structures instead.

Protect your code by:

def main():
    ...

if __name__ == '__main__':
    main()

that way your code don't get executed if you import your code into another file.

Instead of choosing a random chore, you could just shuffle them all beforehand, that way the randomness task is done.

chores = chores[:]
shuffle(chores)

Another way to call your function then to send in a None

def assign_chores(chore_list, prev_assignment_data, people_data):
    ...

prev_assignment_data = None
    assigned_chores = assign_chores(chore_list, prev_assignment_data,
                                    people_data)

is to define it in your function

def assign_chore(chores: list, persons: dict, prev_assignment: dict=None):
    ...

now you can call it like

people_data = assign_chore(chores, people_data)
people_data = assign_chore(chores, people_data, some_previous_data)

When I broke down your code I did it like this:

from random import shuffle

def assign(chores: list, persons: dict):
    for key, person in enumerate(persons):
        persons[person]["chores"] = chores[key]
    return persons

def get_shuffled_chores(chores: list, persons: dict):
    chores = chores[:]
    shuffle(chores)
    chores_dist = [[] for _ in range(len(persons))]
    for key, i in enumerate(range(len(chores))):
        chores_dist[key % len(persons)] += [chores.pop()]
    return chores_dist

def assign_without_regard(chores: list, persons: dict):
    return assign(get_shuffled_chores(chores, persons), persons)

def is_repetitive(news: list, olds: list):
    for new, old in zip(news, olds):
        if any(chore in old for chore in new):
            return False
    return True

def assign_with_regard(chores: list, prev_assigment: dict, persons: dict):
    prev_chores = [person["chores"] for person in prev_assigment.values()]
    new_chores = get_shuffled_chores(chores, persons)
    return assign(new_chores, persons) \
        if not is_repetitive(new_chores, prev_chores) \
        else assign_chore(chores, prev_assigment, persons)

def assign_chore(chores: list, persons: dict, prev_assignment: dict=None):
    if prev_assignment is None:
        return assign_without_regard(chores, persons)
    return assign_with_regard(chores, prev_assignment, persons)

def main():
    chores = ["hoovering", "dusting", "wash dishes", "tidy garden",
              "clean windows", "empty dishwasher", "dust", "cook"]

    people_data = {"David": {"email": "example@gmail.com",
                             "chores": []},
                   "Mark": {"email": "example@gmail.com",
                            "chores": []},
                   "Anaya": {"email": "example@gmail.com",
                             "chores": []},
                   "Cooper": {"email": "example@gmail.com",
                              "chores": []}
                   }

    people_data = assign_chore(chores, people_data)
    print(people_data)
    print(assign_chore(chores, people_data, people_data))

if __name__ == '__main__':
    main()

StackExchange Code Review Q#148747, answer score: 2