+ Start a Discussion
CB312CB312 

Moving my SOQL outside the for Loop

Hi All,

 

I keep hitting the SOQL limit with this trigger, I know I need to move the query outside the For loop, but I can't figure out how to do that while still accomplishing my main goal.

 

Apex governor limit warning

Operation: update.Opportunity

Caused the following Apex resource warnings:

Number of SOQL queries: 100 out of 100


		
		

 

 

 

 

trigger updateDivisions on Opportunity (before update) {
    for(Opportunity o : trigger.new){
        /*
            I query for the division where the Name is equal to the picklist value selected on the Opportunity.
            It is important to query and store to a list (even though you are only expecting one value to be returned)
            because if you do not store it to a list and the query returns nothing, the trigger will break
            and throw an error message. 
        */
        List<Division__c> myDivision = [select Id from Division__c where Name =: o.Division__c limit 1];
        
        /*
            I have created a lookup on the Opportunity object to the Division__c object so that when you have the 
            Id of the division you queried for, you can relate it back to that record on the opportunity record
            detail page.
        */
        if(myDivision.size() > 0) {
            /*
                By specifying myDivision[0] you grab the first element of the list (in this instance, 
                it is the only element in the list as a result of the delimiter in the SOQL query
            */
            o.Division_Dynamic__c = myDivision[0].Id;
        } else {
            //Assign to a dummy division value for testing to isolate opportunities with an incorrect division
            o.Division_Dynamic__c = 'a0PC0000008CGfs';
        }
    }
}

 

Thanks!

 

Best Answer chosen by Admin (Salesforce Developers) 
bob_buzzardbob_buzzard

I think you want something along the lines of the following.  Caveat emptor - I haven't compiled or tested it, but it should be pretty close:

 

 

trigger updateDivisions on Opportunity (before update) {

    // build a map of opportunities keyed by division name
    Map<String, List<Opportunity>> oppsByDivName=new Map<String, List<Opportunity>>();
    for (Opportunity o : trigger.new)
    {
        // clear out the division name so that we can tell if there was no match
        o.Division_Dynamic__c=null;

        // add to the list of opps with this division - creating the list map entry
        // if required
        List<Opportunity> cand=oppsByDivName.get(o.Division__c);
        if (null==cand)
        {
            cand=new List<Opportunity>();
            oppsByDivName.put(cand, o.Division__c);
        }
        cand.add(o);
    }

    // query all divisions in one go based on the names from the map
    List<Division__c> myDivisions=[select id, Name from Division__c where Name IN :divNames]
    for (Division__c div : myDivisions)
    {
       // find the opportunities with the matching name
       List<Opportunity> opps=oppsByDivName.get(div.name);
       
       if (null!=opps)
       {
        for (Opportunity o : opps)
           {
              o.Division_Dynamic__c = div.Id;
           }
       }
    }

    // now fill in all empty divisions with the placeholder
    for (Opportunity o : trigger.new)
    {
        //Assign to a dummy division value for testing to isolate opportunities with an incorrect division
        o.Division_Dynamic__c = 'a0PC0000008CGfs';
    }
}

 

 

 

All Answers

bob_buzzardbob_buzzard

I think you want something along the lines of the following.  Caveat emptor - I haven't compiled or tested it, but it should be pretty close:

 

 

trigger updateDivisions on Opportunity (before update) {

    // build a map of opportunities keyed by division name
    Map<String, List<Opportunity>> oppsByDivName=new Map<String, List<Opportunity>>();
    for (Opportunity o : trigger.new)
    {
        // clear out the division name so that we can tell if there was no match
        o.Division_Dynamic__c=null;

        // add to the list of opps with this division - creating the list map entry
        // if required
        List<Opportunity> cand=oppsByDivName.get(o.Division__c);
        if (null==cand)
        {
            cand=new List<Opportunity>();
            oppsByDivName.put(cand, o.Division__c);
        }
        cand.add(o);
    }

    // query all divisions in one go based on the names from the map
    List<Division__c> myDivisions=[select id, Name from Division__c where Name IN :divNames]
    for (Division__c div : myDivisions)
    {
       // find the opportunities with the matching name
       List<Opportunity> opps=oppsByDivName.get(div.name);
       
       if (null!=opps)
       {
        for (Opportunity o : opps)
           {
              o.Division_Dynamic__c = div.Id;
           }
       }
    }

    // now fill in all empty divisions with the placeholder
    for (Opportunity o : trigger.new)
    {
        //Assign to a dummy division value for testing to isolate opportunities with an incorrect division
        o.Division_Dynamic__c = 'a0PC0000008CGfs';
    }
}

 

 

 

This was selected as the best answer
CB312CB312

Wow Bob, thanks for the quick reply. Quick question, where are you getting ":divNames" for this SOQL query:

 

List<Division__c> myDivisions=[select id, Name from Division__c where Name IN :divNames

 

Thanks,

Chris

bob_buzzardbob_buzzard

From my secret stash of code that I forgot to paste in!

 

Divnames is the keyset from the oppsByDivName map - just before that line it needs:

 

 

Set<String> divNames=oppsByDivName.keySet();

 

 

I've just spotted another typo too - earlier on where building the map the parameters to the put are the wrong way around:

 

 

oppsByDivName.put(cand, o.Division__c);

 

should be:

 

 

oppsByDivName.put(o.Division__c, cand);

 

 

 

 

CB312CB312

Thanks, the code is kicking out: "You do not have the level of access necessary to perform the operation you requested. Please contact the owner of the record or your administrator if access is necessary."

Below is the code being used:

 

trigger updateDivisions on Opportunity (before update) {

    // build a map of opportunities keyed by division name
    Map<String, List<Opportunity>> oppsByDivName=new Map<String, List<Opportunity>>();
    Set<String> divNames = oppsByDivName.keySet();
    for (Opportunity o : trigger.new)
    {
        // clear out the division name so that we can tell if there was no match
          o.Division_Dynamic__c=null;

        // add to the list of opps with this division - creating the list map entry
        // if required
        
        List<Opportunity> cand=oppsByDivName.get(o.Division__c);
        if (null==cand)
        {
            cand=new List<Opportunity>();
            oppsByDivName.put(o.Division__c, cand);
        }
        cand.add(o);
    }

    // query all divisions in one go based on the names from the map
    List<Division__c> myDivisions=[select id, Name from Division__c where Name IN :divNames];
    for (Division__c div : myDivisions)
    {
       // find the opportunities with the matching name
       List<Opportunity> opps=oppsByDivName.get(div.name);
       
       if (null!=opps)
       {
        for (Opportunity o : opps)
           {
              o.Division_Dynamic__c = div.Id;
           }
       }
    }

    // now fill in all empty divisions with the placeholder
    for (Opportunity o : trigger.new)
    {
        //Assign to a dummy division value for testing to isolate opportunities with an incorrect division
        o.Division_Dynamic__c = 'a0PC0000008CGfs';
    }
}

 any ideas?

 

bob_buzzardbob_buzzard

This line of code:

 

 

 Set<String> divNames = oppsByDivName.keySet();

 

 

needs to be placed just above the query where divNames is used, e.g. immediately above:

 

 

 
 // query all divisions in one go based on the names from the map
    List<Division__c> myDivisions=[select id, Name from Division__c where Name IN :divNames];

 

Otherwise the keyset will be empty as the map has just been instantiated.

 

 

 

 

CB312CB312

Ok, So I figured out why it was erroring the insufficent privdleges error. The hardcoded ID i used as a catch-all was from production, not sandbox so it was trying to insert a record that didn't exisit. After changing the hard coded ID back to the sandbox catch all, the code runs and exectues, however it dumps in the catchall, and not in the division. Even with the change to the code:

 Set<String> divNames = oppsByDivName.keySet();

 

Ideas why it might not be grabbing the ID?

 

bob_buzzardbob_buzzard

Did you see my revised post  above - I went away from the permissions route and identified that divnames was being set up too early.

bob_buzzardbob_buzzard

Here is is reproduced:

 

This line of code:

 

 

 Set<String> divNames = oppsByDivName.keySet();

 

 

needs to be placed just above the query where divNames is used, e.g. immediately above:

 

 

 
 // query all divisions in one go based on the names from the map
    List<Division__c> myDivisions=[select id, Name from Division__c where Name IN :divNames];

 

Otherwise the keyset will be empty as the map has just been instantiated.

 

CB312CB312

Bob - thank you for all the help! I figured out why it was dumping the error, it was running through the last for loop, i needed to put an if statement above the for loop for records with a null value. So happy to have this up and running.

 

Thanks again!

-Chris

 

ps. Love the blog. I found it a couple weeks ago and have been crawling over all the links you post :)

bob_buzzardbob_buzzard

And thank you in return for those kind words.  Its nice to know the efforts are appreciated.