function readOnly(count){ }
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
+ Start a Discussion
franky@gmail.comfranky@gmail.com 

My first trigger, and potential governor limits issue

I had the pleasure of writing my first trigger today.  I'm not new to programming, just new to the APEX language.  In creating this trigger, I tried to adhere as much as I can to some of the best practices outlined in many of the resources available.  I do however have some concerns about my code, specifically with regards to what I'm reading about governor limits.

 

Here's a brief background into what the business process is and why the trigger is necessary: We have a need to keep track of the campaigns a lead or contact takes part of.  We have a specific text field in the Lead and Contact table where a list of campaign names of the campaigns they are a part of will be stored.  This field will be updated each time a new campaign member record is created or modified for this lead or contact. 

 

The trigger is on the Campaign Member table, and basically, each time the trigger fires, the field (on the Lead or Contact record, depending on which one it is) will be re-calculated based on all of the campaigns that person is a part of.  The guts of the trigger will get a list of all the Campaign Member records for the Lead or Contact (of this current Campaign Member record), from a SOQL statement.  This SOQL statement is embedded in the "for" loop, which may break if the governor limit for SOQL queries are reached, in this case, 100 SOQL queries.  So, if we have a bulk upload of Campaign member records using say a data loading tool of some sort, it seems to me that using this trigger, we'd hit the governor limit real quick.  How can this be improved so that we do not reach the governor limits?

 

Any help is appreciated!

 

Thanks,

Frank

 

Trigger code below:

==================================================

 

trigger UpdateProspectWithCampaignName on CampaignMember (after insert, after update, after delete) {
    if (Trigger.isAfter && (Trigger.isInsert || Trigger.isUpdate)) {      
        // Iterate over each CampaignMember record
        for (CampaignMember newCM : Trigger.new) {  
            string mycampaigns = '';         
            if (!string.isEmpty(newCM.ContactId)) { //If the Prospect is a Contact record
                Contact ContactToMod = [SELECT Id, Name, Test_CL__c, (SELECT Campaign.Name FROM CampaignMembers)
                FROM Contact
                WHERE Id = :newCM.ContactId];
                // Access child records.
                List<CampaignMember> cms = ContactToMod.CampaignMembers;
                for(CampaignMember thisone : cms){
                    mycampaigns += thisone.Campaign.Name + ';';
                }
                mycampaigns = mycampaigns.removeEnd(';');
                ContactToMod.Test_CL__c = mycampaigns;
                update ContactToMod;                         
            } else  { //If the Prospect is a Lead record
                Lead LeadToMod = [SELECT Id, Name, Test_CL__c, (SELECT Campaign.Name FROM CampaignMembers)
                FROM Lead
                WHERE Id = :newCM.LeadId];
                // Access child records.
                List<CampaignMember> cms = LeadToMod.CampaignMembers;
                for(CampaignMember thisone : cms){
                    mycampaigns += thisone.Campaign.Name + ';';
                }
                mycampaigns = mycampaigns.removeEnd(';');
                LeadToMod.Test_CL__c = mycampaigns;
                update LeadToMod;           
            }    
        }
    }
}

 

 

Best Answer chosen by Admin (Salesforce Developers) 
Arun MKArun MK

 

Make use of the Collection types provided by salesforce like Lists, Sets and Maps to achieve the functionality you desire for handling bulk triggers.

 

Try this one:

 

if (Trigger.isAfter && (Trigger.isInsert || Trigger.isUpdate)) {

    // Iterate over each CampaignMember record

    

    Set<Id> setOfContactIds = new Set<Id>();

    Set<Id> setOfLeadIds = new Set<Id>();

    string mycampaigns = '';

    List<Contact> updateContactList = new List<Contact>();

    List<Lead> updateLeadList = new List<Lead>();

 

    for (CampaignMember newCM : Trigger.new) {

        if (!string.isEmpty(newCM.ContactId))

            setOfContactIds.add(newCM.ContactId);

        else

            setOfLeadIds.add(newCM.LeadId);

    }

    

    for(Contact ContactToMod = [SELECT Id, Name, Test_CL__c, (SELECT Campaign.Name FROM CampaignMembers) FROM Contact WHERE Id in :setOfContactIds]){

        mycampaigns = '';

        for(CampaignMember thisone : ContactToMod.CampaignMembers){

            mycampaigns += thisone.Campaign.Name + ';';

        }

        ContactToMod.Test_CL__c = mycampaigns.removeEnd(';');

        updateContactList.add(ContactToMod);

    }

    

    update updateContactList;

    

    for(Lead LeadToMod = [SELECT Id, Name, Test_CL__c, (SELECT Campaign.Name FROM CampaignMembers) FROM Lead WHERE Id in :setOfLeadIds){

        mycampaigns = '';

        for(CampaignMember thisone : ContactToMod.CampaignMembers){

            mycampaigns += thisone.Campaign.Name + ';';

        }

        LeadToMod.Test_CL__c = mycampaigns.removeEnd(';');

        updateLeadList.add(LeadToMod);

    }

    

    update updateLeadList;

}

 

Regards,

Arun,

All Answers

k_bentsenk_bentsen

The tried and true method is to build a list of Ids within your for loop, then outside the loop obtain the records you need to update based on the list of Ids with a single SOQL query, instead of a query for each iteration of the loop.

 

Check out #2 on the following page: http://wiki.developerforce.com/page/Apex_Code_Best_Practices

Arun MKArun MK

 

Make use of the Collection types provided by salesforce like Lists, Sets and Maps to achieve the functionality you desire for handling bulk triggers.

 

Try this one:

 

if (Trigger.isAfter && (Trigger.isInsert || Trigger.isUpdate)) {

    // Iterate over each CampaignMember record

    

    Set<Id> setOfContactIds = new Set<Id>();

    Set<Id> setOfLeadIds = new Set<Id>();

    string mycampaigns = '';

    List<Contact> updateContactList = new List<Contact>();

    List<Lead> updateLeadList = new List<Lead>();

 

    for (CampaignMember newCM : Trigger.new) {

        if (!string.isEmpty(newCM.ContactId))

            setOfContactIds.add(newCM.ContactId);

        else

            setOfLeadIds.add(newCM.LeadId);

    }

    

    for(Contact ContactToMod = [SELECT Id, Name, Test_CL__c, (SELECT Campaign.Name FROM CampaignMembers) FROM Contact WHERE Id in :setOfContactIds]){

        mycampaigns = '';

        for(CampaignMember thisone : ContactToMod.CampaignMembers){

            mycampaigns += thisone.Campaign.Name + ';';

        }

        ContactToMod.Test_CL__c = mycampaigns.removeEnd(';');

        updateContactList.add(ContactToMod);

    }

    

    update updateContactList;

    

    for(Lead LeadToMod = [SELECT Id, Name, Test_CL__c, (SELECT Campaign.Name FROM CampaignMembers) FROM Lead WHERE Id in :setOfLeadIds){

        mycampaigns = '';

        for(CampaignMember thisone : ContactToMod.CampaignMembers){

            mycampaigns += thisone.Campaign.Name + ';';

        }

        LeadToMod.Test_CL__c = mycampaigns.removeEnd(';');

        updateLeadList.add(LeadToMod);

    }

    

    update updateLeadList;

}

 

Regards,

Arun,

This was selected as the best answer
franky@gmail.comfranky@gmail.com
Arun,

Absolutely. This is pretty much exactly what I ended up doing. The only real difference is that I used a List of Id's instead of a Set of Id's.

I was able to do more in-depth research on the use of lists and sets and maps in triggers. I've since moved all the SOQL statements outside of any loops! Thanks for your suggestion!

Thanks to all for their input!

-Frank