CODEWITHSUNDEEP
python
JP29
JP29

Reputation: 625

Alternative to nested `for` loops in Python

I am trying to compare a list of forbidden folders read from a file. But we have to check if the task has the parent ID of the folder and then if that folder matches the forbidden folder. The lists I am looping through can contain many items.

for task in tasks:
    #Check the task is on their timesheet
    if task["id"] == entry["item_id"]:
        for folder in folders:
            #See if the task is inside the folder
            if task["parent_id"] == folder["id"]:
                for forbiddenFolder in forbiddenFolders:
                    #If the folder is on the list
                    if forbiddenFolder == folder["name"]:
                        body_of_email +=( "Using a task within a folder that is not permiited " + forbiddenFolder + "\r\n" )
                        folder["name"]
                        break

This code uses three nested for loops, which might be slow. Can I make this more efficient?

Upvotes: 0

Views: 3774

Answers (4)

Down the Stream
Down the Stream

Reputation: 697

taking a step back from the intricate details in the original inquiry, and put attention on just some "Alternatives to nested for loops" in general, how about the following:

tasks = {
    'design': 'later',
    'testing': 'desired',
    'vacation': 'fall'
}

folders = {
    'summer': 'red',
    'fall': 'gold',
    'winter': 'white',
    'spring': 'green'
}

def apply_some_logic(a, b, c, d):
    return '_'.join([a, b, c, d]) if b == c else (a, b, c, d)


if __name__ == '__main__':
    results = (apply_some_logic(a, b, c, d) for a, b in tasks.items() for c, d in folders.items())
    for result in results:
        print(result)


$ python3.6 --version
Python 3.6.4

output:
('design', 'later', 'Summer', 'Red')
('design', 'later', 'Fall', 'Gold')
('design', 'later', 'Winter', 'White')
('design', 'later', 'Spring', 'Green')
('implementation', 'urgent', 'Summer', 'Red')
('implementation', 'urgent', 'Fall', 'Gold')
('implementation', 'urgent', 'Winter', 'White')
('implementation', 'urgent', 'Spring', 'Green')
('testing', 'desired', 'Summer', 'Red')
('testing', 'desired', 'Fall', 'Gold')
('testing', 'desired', 'Winter', 'White')
('testing', 'desired', 'Spring', 'Green')
('vacation', 'Fall', 'Summer', 'Red')
vacation_Fall_Fall_Gold
('vacation', 'Fall', 'Winter', 'White')
('vacation', 'Fall', 'Spring', 'Green')

Upvotes: 0

user1444165
user1444165

Reputation:

A method which lets you keep the original datastructures is the following:

for task in (t for t in tasks if entry["item_id"]==t["id"]):
    for folder in (f for f in folders if task["parent_id"]==f["id"]):
        for forbiddenFolder in (ff for ff in forbiddenFolders if ff==folder["name"]):
            body_of_email += "Using a task within a folder that is not permitted %s \r\n"% forbiddenFolder
            break

This makes use of generators, which are very memory-efficient and thus preserve speed, and conditional for-loops of the form x for x in range(y) if condition. I recommend taking a look at both.

Upvotes: 1

Rodrigo Queiro
Rodrigo Queiro

Reputation: 1336

What you're trying to do here is look up an item (task, folder) based on the id. Python's dictionaries provide an easy way to do this. You will only save if you'll be doing the searches several times (e.g. if there are many tasks with the same id, or if you'll be running the function several times).

Additionally, for forbiddenFolders you just have a list of names (you're not looking up an item, you're just checking if it's present) for which Python's sets are suitable.

Anyway, here is how you build the dictionaries and sets:

tasks_dict = dict((task['id'], task) for task in tasks)
folders_dict = dict((folder['id'], folder) for folder in folders)
forbidden_folders_set = set(forbiddenFolders)

Now, task = tasks_dict[id] is a task such that task['id'] == id, and similarly for folders, so you can replace the loops above with these expressions. The set doesn't allow this, but it allows you to check for presence with folder in forbidden_folders_set.

(Bear in mind that each of those dict(...) operations may take longer than running through one of the for loops above, but they are an investment for faster lookup in future.)

if entry['item_id'] in tasks_dict:
    task = tasks_dict[entry['item_id']]
    if task['parent_id'] in folders_dict:
        folder = folders_dict[task['parent_id']]
        if folder in forbidden_folders_set:
            body_of_email += ...

The x in y and ..._dict[x] operations above are very efficient.

Upvotes: 2

Ansari
Ansari

Reputation: 8218

You can reduce the number of lines of code and make it easier to understand like this:

tasks_id = [task in tasks if task["id"] == entry["item_id"]]
folders_dict = dict()
for folder in folders:
    folders_dict[folder["id"]] = folder

for task in tasks_id:
    if task["parent_id"] in folders_dict.keys() and folder_dict[task["parent_id"]] in forbiddenFolders:
        body_of_email +=( "Using a task within a folder that is not permiited " + forbiddenFolder + "\r\n" )

Upvotes: 3

Related Questions