3

I am facing CPU Run time exception while update opportunity through Data Loader. Because of my rollup trigger. How to avoid for loop inside a for loop?

Trigger:

trigger BudgetAllocationRollup on Budget_Allocation__c (after delete, after insert, after update) {

    Set<Id> Opportunityids=new Set<Id>();
    Map<id,Opportunity__c> mapOpportunity=new Map<id,Opportunity__c>();
    Map<id,List<Budget_Allocation__c>>  mapBudget=new Map<id,List<Budget_Allocation__c>>();
     if(Trigger.isInsert || Trigger.isUpdate){
        List<Budget_Allocation__c> lstBudget=[select id,name,Budget__c,Opportunity1__c ,Opportunity_Flag__c from Budget_Allocation__c where id in:Trigger.newMap.keyset()];
        for(Budget_Allocation__c budget:lstBudget){
             Opportunityids.add(budget.Opportunity1__c );
        }
        List<Opportunity__c> opplst=[select id,name,Budget_Allocation_Total__c,(select Opportunity1__c ,id,Budget__c,name,Opportunity_Flag__c from Budget_Allocations__r) from Opportunity__c where id in:Opportunityids];
        for(Opportunity__c opp:opplst){
            mapOpportunity.put(opp.id,opp);
            mapBudget.put(opp.id,opp.Budget_Allocations__r);
        }
        AggregateResult i=[select sum(Budget__c) from Budget_Allocation__c where Opportunity1__c in:mapOpportunity.keyset()];
        for(Id id:mapOpportunity.keyset()){

                mapOpportunity.get(id).Budget_Allocation_Total__c=0;
                decimal total=0;
                for(Budget_Allocation__c budget: mapBudget.get(id)) {
                    if(budget.Budget__c!=null && budget.Opportunity_Flag__c==true)

                        mapOpportunity.get(id).Budget_Allocation_Total__c=mapOpportunity.get(id).Budget_Allocation_Total__c+budget.Budget__c;
                }   

        }
        if(mapOpportunity!=null && mapOpportunity.size()>0){
          try{
                update mapOpportunity.values();
          }
          catch(Exception e){

          }
        }
     }
    if(Trigger.isDelete){

            List<Budget_Allocation__c> lstBudget=[select id,name,Budget__c,Opportunity1__c ,Opportunity_Flag__c from Budget_Allocation__c where id in:Trigger.oldMap.keyset()];
            //system.debug('--lstBudget--->'+lstBudget); 
            if(lstBudget!=null){
            for(Budget_Allocation__c budget:lstBudget){
                 Opportunityids.add(budget.Opportunity1__c );
            }
       }
            List<Opportunity__c> opplst=[select id,name,Budget_Allocation_Total__c,(select Opportunity1__c ,id,Budget__c,name from Budget_Allocations__r) from Opportunity__c where id not in:Opportunityids];
                for(Opportunity__c opp:opplst){
                    mapOpportunity.put(opp.id,opp);
                     mapBudget.put(opp.id,opp.Budget_Allocations__r);
                }
            AggregateResult i=[select sum(Budget__c) from Budget_Allocation__c where Opportunity1__c in:mapOpportunity.keyset()];
            for(Id id:mapOpportunity.keyset()){
                mapOpportunity.get(id).Budget_Allocation_Total__c=0;
                decimal total=0;
                for(Budget_Allocation__c budget: mapBudget.get(id)) {
                    if(budget.Budget__c!=null)

                        mapOpportunity.get(id).Budget_Allocation_Total__c=mapOpportunity.get(id).Budget_Allocation_Total__c+budget.Budget__c;
                }   

            }
            if(mapOpportunity!=null && mapOpportunity.size()>0){
              try{
                    update mapOpportunity.values();
              }
              catch(Exception e){

              }
            }
     }

}
  • 1
    Please first explain what are you trying to achieve with this trigger, then someone may help you technically. – Jarvis Dec 25 '15 at 13:37
6

For inside FOR loop is not causing this issue as there is no SOQL inside FOR loop.

There are couple of things i thought will reduce your CPU times

1)Use Inline SOQL rather than looping over the list.This consumes less CPU times

The below consumes more CPU time since the entire list is queried and iterated

List<Budget_Allocation__c> lstBudget=[select id,name,Budget__c,Opportunity1__c ,Opportunity_Flag__c from Budget_Allocation__c where id in:Trigger.newMap.keyset()];
    for(Budget_Allocation__c budget:lstBudget){
         Opportunityids.add(budget.Opportunity1__c );
    }

Inline SOQL reduces the CPU time as well as Heap

//add inline SOQL
    for(Budget_Allocation__c budget:[select id,name,Budget__c,Opportunity1__c ,Opportunity_Flag__c from Budget_Allocation__c where id in:Trigger.newMap.keyset()]){
         Opportunityids.add(budget.Opportunity1__c );
    }

2)Your aggregate SOQL is no where used so i have removed the same from trigger .

Below is revised trigger with my comment added wherever modified

trigger BudgetAllocationRollup on Budget_Allocation__c (after delete, after insert, after update) {

Set<Id> Opportunityids=new Set<Id>();
Map<id,Opportunity__c> mapOpportunity=new Map<id,Opportunity__c>();
Map<id,List<Budget_Allocation__c>>  mapBudget=new Map<id,List<Budget_Allocation__c>>();
 if(Trigger.isInsert || Trigger.isUpdate){
  //add inline SOQL
    for(Budget_Allocation__c budget:[select id,name,Budget__c,Opportunity1__c ,Opportunity_Flag__c from Budget_Allocation__c where id in:Trigger.newMap.keyset()]){
         Opportunityids.add(budget.Opportunity1__c );
    }
    //add inline SOQL
    for(Opportunity__c opp:[select id,name,Budget_Allocation_Total__c,(select Opportunity1__c ,id,Budget__c,name,Opportunity_Flag__c from Budget_Allocations__r) from Opportunity__c where id in:Opportunityids]){
        mapOpportunity.put(opp.id,opp);
        mapBudget.put(opp.id,opp.Budget_Allocations__r);
    }
    //Removed Aggregate SOQL
    for(Id id:mapOpportunity.keyset()){

            mapOpportunity.get(id).Budget_Allocation_Total__c=0;
            decimal total=0;
            for(Budget_Allocation__c budget: mapBudget.get(id)) {
                if(budget.Budget__c!=null && budget.Opportunity_Flag__c==true)

                    mapOpportunity.get(id).Budget_Allocation_Total__c=mapOpportunity.get(id).Budget_Allocation_Total__c+budget.Budget__c;
            }   

    }
   // if(mapOpportunity!=null && mapOpportunity.size()>0){
      try{
            update mapOpportunity.values();
      }
      catch(Exception e){

      }
    //}
 }
if(Trigger.isDelete){
  //add inline SOQL
        for(Budget_Allocation__c budget:[select id,name,Budget__c,Opportunity1__c ,Opportunity_Flag__c from Budget_Allocation__c where id in:Trigger.oldMap.keyset()]){
             Opportunityids.add(budget.Opportunity1__c );
        }
        //add inline SOQL
        for(Opportunity__c opp:[select id,name,Budget_Allocation_Total__c,(select Opportunity1__c ,id,Budget__c,name from Budget_Allocations__r) from Opportunity__c where id not in:Opportunityids]){
            mapOpportunity.put(opp.id,opp);
             mapBudget.put(opp.id,opp.Budget_Allocations__r);
        }
         //Removed Aggregate SOQL
        for(Id id:mapOpportunity.keyset()){
            mapOpportunity.get(id).Budget_Allocation_Total__c=0;
            decimal total=0;
            for(Budget_Allocation__c budget: mapBudget.get(id)) {
                if(budget.Budget__c!=null)

                    mapOpportunity.get(id).Budget_Allocation_Total__c=mapOpportunity.get(id).Budget_Allocation_Total__c+budget.Budget__c;
            }   

        }
      //  if(mapOpportunity!=null && mapOpportunity.size()>0){
          try{
                update mapOpportunity.values();
          }
          catch(Exception e){

          }
    //    }
   }

}
| improve this answer | |
  • That doesn't remove For loop inside a For loop – Jarvis Dec 25 '15 at 13:56
  • 1
    For inside for loop is not causing the CPU limit here . – Mohith Shrivastava Dec 25 '15 at 14:01
  • if(mapOpportunity!=null...? Why would it be null? – sfdcfox Dec 25 '15 at 14:55
  • Ah..That's good catch ..unnecessary if would block the CPU as well and reducing those will reduce CPU times – Mohith Shrivastava Dec 25 '15 at 14:59
  • 1
    It's worth noting that a for loop inside a for loop can absolutely cause CPU time to run up if you're iterating over large sets. Some options are replacing loops with maps wherever you can, using smaller batches, and inlining SQOL. – Christian Carter Dec 25 '15 at 20:48
2

Use following technique to reduce For loop. and it is used to reduce dependent queries in For loop. PS> This is for general concept, you can use it according to your logic, as you didn't mentioned what you are trying to achieve.

Technique 1: To Reduce for loops

Map<id,Opportunity__c> mapOpportunity=new Map<id,Opportunity__c>();
//^ this can be write as below, as you are not having conditions
Map<Id, Opportunity__c> mapOpportunity = new Map<Id, Opportunity__c>([select id,name,Budget_Allocation_Total__c,(select Opportunity1__c ,id,Budget__c,name,Opportunity_Flag__c from Budget_Allocations__r) from Opportunity__c where id in:Opportunityids]);

Technique 2:

Suppose your query is dependent on another object's id.,,,

Let's see the problem first:

for(objectDemo oTemp : listDemo)
{
   objectDemo1 oDemo1Temp = [select id from objectDemo1 where id =: oTemp.id];
   //processing of objectDemo1
}

Proposed Solution:

//taking list of objectDemo1 so that we don't have to repeatedly call it
Map<Id, objectDemo1> mapDemo1 = new Map<Id, objectDemo1>([select id from objectDemo1]);
for( objectDemo oTemp: listDemo)
{
  objectDemo1 oDemo1Temp = mapDemo1.get(oTemp.id);
  //processing of objectDemo1
}
| improve this answer | |

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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