Take the 2-minute tour ×
Salesforce Stack Exchange is a question and answer site for Salesforce administrators, implementation experts, developers and anybody in-between. It's 100% free, no registration required.

I have a Web Service class. When I call the method from this class, I get "Apex CPU time limit exceeded" error. The for loops are causing this trouble. Here's my not efficient code:

List<Task> tasks = [SELECT Id, WhoId FROM Task];
List<Contact> contacts = [SELECT Id FROM Contact];

for (Task t : tasks){
    for(Contact c : contacts){
        if (t.WhoId == c.Id){
            //some logic here   
        } 
    }
}

This piece of code matches the related Task and Contact objects. How to make it more efficient? Is there a way to reduce the number of loops here to one?

share|improve this question
add comment

3 Answers

up vote 6 down vote accepted

Use a Map instead:

List<Task> tasks = [SELECT Id ... FROM Task];
Map<Id, Contact> contacts = new Map<Id, Contact>([SELECT Id ... FROM Contact]);

for (Task t : tasks){
    Contact c = contacts.get(t.WhoId);
    if(c != null) {
        //some logic here   
    }
}

This technique was originally used to avoid script limits, and it's equally effective to avoid timeout limits.

Edit

As stated by Mike, you should make sure your filters are also correctly limiting records to avoid heap limits and query limits, and your field list should include only the fields necessary to complete the task.

Task[] tasks = [SELECT Id ... FROM Task WHERE ...];
Map<Id, Contact> contacts = new Map<Id, Contact>();
for(Task record: tasks) {
    if(record.WhoId != null && record.WhoId.getSObjectType() == Contact.SObjectType) {
        contacts.put(record.WhoId, null);
    }
}
contacts.putAll([SELECT Id ... FROM Contact WHERE Id IN :contacts.keySet()]);
share|improve this answer
add comment

The problem stems from your SOQL queries; they're fetching ALL Tasks and ALL Contacts. Here's one way to improve that:

  1. Filter your tasks to only those relevant
  2. Retrieve those WhoId that belong to Contacts by checking their prefix
  3. Fetch the necessary Contacts

    List<Task> tasks = [SELECT Id, WhoId FROM Task WHERE /* INSERT FILTER */];
    List<id> taskWhoId = new List<id>();
    for (Task t : tasks){
      // Only take those matched to a Contact
      if (t.WhoId.startsWith('003'){
        taskWhoId.add(t.WhoId);
      } 
    }
    
    Map<Id,Contact> contacts = new Map<id, Contact>([SELECT Id FROM Contact WHERE ID in :taskWhoId]);    
    for (Task t : tasks){
      if contacts.containsKey(t.WhoId){
        Contact taskContact = contacts.get(t.WhoId);
        // Do something with the Task and Contact
      }
    }
    

As Phil points out, you could also combine this into one query, if you can filter on Contacts instead of Tasks (that depends on your requirements).

share|improve this answer
1  
Actually, the queries aren't the topic of this question-- they're asking about eliminating the inner loop. That aside, some notes: (1) you didn't check if t.whoId is null first, so NPE if there is no contact, and (2) you really should use t.WhoId.getSObjectType() == Contact.SobjectType instead of checking the key prefix, since, in theory, the key could change and this code would no longer operate as expected (I know it "shouldn't", but as an external developer, one cannot guarantee that). –  sfdcfox Dec 12 '13 at 18:20
    
Both are good points. I plain forgot the first problem and never considered using getSObjectType in my comparisons before; I'll star using that in the future. –  Mike Chale Dec 12 '13 at 19:02
add comment

My crack at it:

List<Task> tasks = [SELECT Id, WhoId FROM Task];
map<id,Contact> contactMap = new map<id,Contact>([SELECT Id FROM Contact]);

for (Task t : tasks){
    Contact thisTasksContact = contactMap.get(t.whoId);
    // do stuff
}
share|improve this answer
    
A bit more information about your solution would be great. –  mast0r Dec 12 '13 at 20:51
    
which part? the map construction? or why the map? –  amatorVitae Dec 12 '13 at 20:55
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.