You need to sign in to do that
Don't have an account?
Can someone explain how to best avoid using SOQL statements in a for loop in a trigger?
The essential information is:
Obect 1 has a master-detail relationship with Account , but is one-to-one with Account.
Object 1 also has a master-detail relationship with Opportunity that is one-to-many, (many Object1's to one Opportunity).
Each Opportunity has fields that need to be acted on in a for loop and then Object 1 is updated after that loop.
The original trigger essentially does the following:
create Set accountIDs from trigger.new
List<Object1> objList = [select Account__c ... from sObject1 where Account__c in :accountIDs]
for (Object1 acc : accountIDs) {
// some code
List<Opportunity> oppList = [select AccountID ... from sObject1 where AccountID = :accountIDs]
// some more code
change fields in objList
} // end for
upsert objList
I need to take the SOQL statement out of the for loop, so the revised trigger does this:
create Set accountIDs from trigger.new
List<Object1> objList = [select Account__c ... from sObject1 where Account__c in :accountIDs]
// next line uses "in :accountIDs" instead of "= :accountIDs"
List<Opportunity> oppList = [select AccountID ... from sObject1 where AccountID in :accountIDs]
for (Object1 acc : accountIDs) {
// some code
for (Opportunity opp : oppList) {
if (opp.AccountID == acc.Id) {
// some code
} // end if
} // end inner for
// some more code
change fields in objList
} // end outer for
upsert objList
The problem with the structure of the second trigger is that I am iterating over all the Opportunities every time, to find the relevant Opportunities with AccountID = acc.Id, which seems terribly inefficient. I considered removing the "used" Opportunites after they have been processed, but then I'm still iterating over many irrelevant Opportunities on each loop.
There must be a better solution. Can anyone help? Thanks.
Sure, it could be more efficient. Consider this for example:
You could then pull each list out of the Map as required. I'm not sure what gains you'd get, but it might be considered less sloppy.
Wes
All Answers
Hey
In the second piece of code you're iterating over the same number of opportunities, but without having to fetch anythign from the DB so it's more efficient. Both sections of code filter with the same criteria.
Wes
But from a coding point of view, it seems sloppy. There's got to be a more efficient way to code this.
Sure, it could be more efficient. Consider this for example:
You could then pull each list out of the Map as required. I'm not sure what gains you'd get, but it might be considered less sloppy.
Wes
Thanks, I like your solution.