function readOnly(count){ }
Sign Up ›
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
Learn More

Browse by Topic
ShowAll Questionssorted byDate Posted
+ Start a Discussion
ChickenOrBeefChickenOrBeef 

Need help writing more efficient loops

Hey everyone,

I've been running into "CPU Time Limit" errors recently, and I believe it has something to do with my sloppy use of loops within loops. So with that said, here's an example of how I normally handle loops within loops. For example, this could be a trigger where I want to simply count the number of Opportunities present under each Account:
  
Set<String> accountsInTrigger = new Set<String>();
        
        FOR(Opportunity o : opps){
            
            IF(o.AccountId != NULL){
                
                accountsInTrigger.add(o.AccountId);
                
            }
            
        }
        
        List<Opportunity> relatedOpps = [SELECT
                                            Id,AccountId
                                         FROM
                                            Opportunity
                                         WHERE
                                            AccountId In :accountsInTrigger];
        
        FOR(String account : accountsInTrigger){
            
            FOR(Opportunity opp : oppsInTrigger){
                
                IF(opp.AccountId == account){
                    
                    /* Insert code here */ 
                    
                }
                
            }
            
        }

I'm assuming that this isn't efficient since this code will loop through every Opp when looping through each Account, regardless if the Opp is attached to that Account or not. Then the IF statement filters down the Opps within each loop, but I'm assuming the code is still looping through every single Opp.

Is there a more efficient way to handle this loop within a loop? Or should I try to avoid loops within loops all together?

Thanks!
-Greg

 
Best Answer chosen by ChickenOrBeef
Venkat PolisettiVenkat Polisetti
Try below code. One thing surprising to me is your CPU limit exception. I have more code than the below in a lot of triggers and I have never seen that. There must be something else that is causing it. But below code should not cause it.
  
List<Opportunty> updatedOpps = new List<Opportunity>();

Account[] accs = [
		SELECT Id, 
			(SELECT Id, Custom_Order_Number__c 
			 FROM	Opportunities 
			 ORDER BY createdDate ASC)
		FROM   Account
		WHERE  Id in :accountsInTrigger];

for (Account a : accs) {

	Integer i = 1;
	for (Opportunity o : accs.Opportunities) {
		o.Custom_Order_Number__c = i++;
	}

	updatedOpps.addAll(a.Opportunities);
}

update updatedOpps;

Hope this helps.

All Answers

Navee RahulNavee Rahul

i like chickens

 

Set<String> accountsInTrigger = new Set<String>();
        
        FOR(Opportunity o : opps){
            
            IF(o.AccountId != NULL){
                
                accountsInTrigger.add(o.AccountId);
                
            }
            
        }
        
        List<Opportunity> relatedOpps = [SELECT
                                            Id,AccountId
                                         FROM
                                            Opportunity
                                         WHERE
                                            AccountId In :accountsInTrigger];
        
        FOR(String account : accountsInTrigger){
            
            FOR(Opportunity opp : oppsInTrigger){
                
                IF(opp.AccountId == account){
                    
                    /* Insert code here  is bad you should run into too much sql Error
					
						Collect the details here some list variable 
					
					*/ 
                    
                }
                
            }
            
        }
		
		/*
		
		 Insert code here Will help your server to rest a little
		
		/*
ChickenOrBeefChickenOrBeef
Hey Navee,

Just to clarify, I didn't mean that I would put DML statements where I put "Insert Code Here", but rather I would just have more code in there doing some work. The actual DML statements would take place outside of the loop. Is that what you were referring to?

Thanks,
Greg
Venkat PolisettiVenkat Polisetti
@ChickenOrBeef,

If you are trying to get a count of Opportunities for an account, go with a Rollup Summary field for that on the Account object.

Where are you trying to use the above code?

Is it in a trigger on the Account object as that is where you would like to get a count of Opps for account?

But anyways, I would use an aggregate query to get the count of Opps for an Account using the below SOQL, if I need to.
  
SELECT count(id)
FROM   Opportunity
WHERE  AccountId in :accountsInTrigger
GROUP by AccountId
Thanks,
Venkat
Venkat PolisettiVenkat Polisetti
I forgot to add accountId to the select list. Here is the final SOQL:
  
SELECT count(Id), AccountId
FROM   Opportunity
WHERE  AccountId in :accountsInTrigger
GROUP by AccountId

 
ChickenOrBeefChickenOrBeef
Hey Venkat,

Actually, what I want to do is provide the ordered number on each opportunity. So if an account has three opps created on these dates:

10/7/14
10/23/14
11/13/14

Then I want to have a custom "Order Number" field be populated like this:

10/7/14 - Order Number = "1"
10/23/14 - Order Number = "2"
11/13/14 - Order Number = "3"

I'm assuming I'd have to loop through the Opps by the Created Date. But as I mentioned in my first post, I'm guessing it would be wrong to loop through every single opportunity when looping through each Account. Does that make sense?

Thanks,
Greg
Venkat PolisettiVenkat Polisetti
Try below code. One thing surprising to me is your CPU limit exception. I have more code than the below in a lot of triggers and I have never seen that. There must be something else that is causing it. But below code should not cause it.
  
List<Opportunty> updatedOpps = new List<Opportunity>();

Account[] accs = [
		SELECT Id, 
			(SELECT Id, Custom_Order_Number__c 
			 FROM	Opportunities 
			 ORDER BY createdDate ASC)
		FROM   Account
		WHERE  Id in :accountsInTrigger];

for (Account a : accs) {

	Integer i = 1;
	for (Opportunity o : accs.Opportunities) {
		o.Custom_Order_Number__c = i++;
	}

	updatedOpps.addAll(a.Opportunities);
}

update updatedOpps;

Hope this helps.
This was selected as the best answer
ChickenOrBeefChickenOrBeef
Thanks Venkat! So going forward, if I want to do a loop within a loop, should I always query the child object within the query of the parent object? So I would loop through the child records that are only attached to that respecitve account, instead of checking every single opportunity to see if it's attached to the account?
Venkat PolisettiVenkat Polisetti
This pattern works better for Parent-Child relationships and it saves you a lot of statements and your process consumes less CPU cycles.

In this case, it makes sense as Opportunity is a child of Account and you want to update each child with a value. Your FOR loop is simple to understand and easy to manipulate.