Salesforce bulkify code - is this not already bulkified?

Question

We have a class in salesforce that is called form a trigger. When using Apex Data Loader this trigger throws an error oppafterupdate: System.LimitException: Too many SOQL queries: 101

I commented out a line of code that calls the following static method in a class we wrote and there are no more errors with respect to the governing limit. So I can verify the method below is the culprit.

I'm new to this, but I know that Apex code should be bulkified, and DML (and SOQL) statements should not be used inside of loops. What you want to do is put objects in a collection and use DML statements against the collection.

So I modified the method below; I declared a list, I added Task objects to the list, and I ran a DML statement on the list. I commented out the update statement inside the loop.

 //close all tasks linked to an opty or lead
     public static void closeTasks(string sId) {
         List<Task> TasksToUpdate = new List<Task>{}; //added this
         List<Task> t = [SELECT Id, Status, WhatId from Task WHERE WhatId =: sId]; //opty
         if (t.isEmpty()==false) {
             for (Task c: t) {
                 c.Status = 'Completed';
                 TasksToUpdate.add(c);  //added this
                 //update c;
             }
         }
         update TasksToUpdate;  //Added this
     }

Why am I still getting the above error when I run the code in our sandbox? I thought I took care of this issue but apparently there is something else here? Please help.. I need to be pointed in the right direction.

Thanks in advance for your assistance

Answer 1

You have "fixed" the update part but the code still fails on the too many SELECTs.

We would need to see your trigger's code but it seems to me you're calling your function in a loop in that trigger. So if say 200 Opportunities are updated, your function is called 200 times and in the function's body you have 1 SOQL... Call it more than 100 times and boom, headshot.

Try to modify the function to pass a collection of Ids:

Set<Id> ids = trigger.newMap().keyset();
betterCloseTasks(ids);

And the improved function could look like this:

public static void betterCloseTasks(Set<Id> ids){
    List<Task> tasksToClose = [SELECT Id
        FROM Task
        WHERE WhatId IN :ids AND Status != 'Completed'];

    if(!tasksToClose.isEmpty()){
        for(Task t : tasksToClose){
            t.Status = 'Completed';
        }
        update tasksToClose;
    }
}

Now you have 1 SOQL and 1 update operation no matter whether you update 1 or hundreds of opportunities. It can still fail on some other limits like max 10000 updated records in one transaction but that's a battle for another day ;)