+ Start a Discussion
Jerry ClifftJerry Clifft 

Help getting SOQL outside of FOR loop

Ok, this is giving me head ache, I have so far been unable to get it correct. The below code works, but I really need to get the SOQL outside of the FOR loop, due to apex govenor limits. Can you assist please?

trigger CaseEquipmentStatus on Case (After Update) {
Set<Id> caseIds = new Set<Id>();
Set<Id> EquipIds = new Set<Id>();
       for (Case c: Trigger.New){
         if (c.Opportunity__c != null && c.Status == 'Request Completed' && c.Origin == 'PRM' )
         caseIds.add(c.Opportunity__c);
         }
         for (Equipment__c Eq: [select id, Opportunity__c, Statis__c from
         Equipment__c where Opportunity__c in :caseIds]) {
         if(eq.Statis__c == 'Programming Change Requested'){
         eq.Statis__c = 'Active';    }
         Update eq;
        } }
Best Answer chosen by Jerry Clifft
Adnubis LLCAdnubis LLC
Here ya go. 

trigger CaseEquipmentStatus on Case (After Update) {
Set<Id> caseIds = new Set<Id>();
Set<Id> EquipIds = new Set<Id>();
    for (Case c: Trigger.New){
         if (c.Opportunity__c != null && c.Status == 'Request Completed' && c.Origin == 'PRM' )
   caseIds.add(c.Opportunity__c);
         }
}
list<Equipment__c> equipment = [select id, Opportunity__c, Statis__c from Equipment__c where Opportunity__c in :caseIds];
for (Equipment__c Eq: equipment) {
         if(eq.Statis__c == 'Programming Change Requested'){
   eq.Statis__c = 'Active';   
   }
    }
Database.update(equipment);
}

All Answers

Adnubis LLCAdnubis LLC
Here ya go. 

trigger CaseEquipmentStatus on Case (After Update) {
Set<Id> caseIds = new Set<Id>();
Set<Id> EquipIds = new Set<Id>();
    for (Case c: Trigger.New){
         if (c.Opportunity__c != null && c.Status == 'Request Completed' && c.Origin == 'PRM' )
   caseIds.add(c.Opportunity__c);
         }
}
list<Equipment__c> equipment = [select id, Opportunity__c, Statis__c from Equipment__c where Opportunity__c in :caseIds];
for (Equipment__c Eq: equipment) {
         if(eq.Statis__c == 'Programming Change Requested'){
   eq.Statis__c = 'Active';   
   }
    }
Database.update(equipment);
}
This was selected as the best answer
Adnubis LLCAdnubis LLC
Just a side note as well you also want to make sure you don't include DML statements in a for loop. I removed the query and I also removed the update by simply updating the entire list of Equipment after the for loop.
Jerry ClifftJerry Clifft
Thanks @Adnubis LLC you are the best! I had been messing with that for 2 days with only a headache as the result.

You're code works great, had an extra "}" on line 8 , I imainge a typo. Thanks again for help!

Peter_sfdcPeter_sfdc
I would suggest one slight refinement to Adnubis' solution, which is that if you are only looking for Equipment__c records that have a particular status value, then add that to the where clause of the query as such: 

trigger CaseEquipmentStatus on Case (After Update) {

Set<Id> caseIds = new Set<Id>();
Set<Id> EquipIds = new Set<Id>();

    for (Case c: Trigger.New){
  if (c.Opportunity__c != null && c.Status == 'Request Completed' && c.Origin == 'PRM' ) {
   caseIds.add(c.Opportunity__c);
  }
}

//why not just add the Statis__c value into the where clause? 
list<Equipment__c> equipment = [select id, Opportunity__c, Statis__c
                                 from Equipment__c
                                 where Opportunity__c in :caseIds AND
                                 Statis__c == 'Programming Change Requested'];
                                
for (Equipment__c eq: equipment) {
   eq.Statis__c = 'Active';  
   }
Database.update(equipment);

}
Adnubis LLCAdnubis LLC
Thats a good improvement Peter, better query less code always better :)
Peter_sfdcPeter_sfdc
By the way, Jerry, you are to be lauded that you didn't just say, "it works" and give up on getting your DML outside of the loop. I have heard many a war story of people who had to clean up bad code like that. Well done! \o/