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
Anthony Zirilli 10Anthony Zirilli 10 

Best Practice: 1 trigger per object

Hi there, 

I made 3 seperate triggers to update 3 different fields on the account level. How do I combine them into one trigger for best practices. Code is below. Thanks for the help.
trigger act_opp_subscrip on Account (before insert, before update) {
    List<Opportunity> oppList = new List<Opportunity>([SELECT Id, Name,AccountId FROM Opportunity WHERE AccountID in : trigger.new 
AND ((RecordType.Name = 'Subscription Renewal' AND Subscription_Status__c = 'Active') OR (RecordType.Name = 'Subscription Prospect' AND Web_Expire_Date__c != null))]);
    Map<Id,Opportunity> accIdwithOpp = new Map<Id,Opportunity>();
    
    for(Opportunity opp : oppList){
        if(!accIdwithOpp.containsKey(opp.AccountId)){
            accIdwithOpp.put(opp.AccountId,opp);
        }
    }
    
    for(Account acc : trigger.new){
        if(accIdwithOpp.containsKey(acc.id)){
            acc.Active_Subscription_Opportunity__c = accIdwithOpp.get(acc.id).id;
        }
        else{
            acc.Active_Subscription_Opportunity__c = null;
        }
    }
}
 
trigger act_opp on Account (before insert, before update) {
    List<Opportunity> oppList = new List<Opportunity>([SELECT Id, Name,AccountId FROM Opportunity WHERE AccountID in : trigger.new 
AND ((RecordType.Name = 'Membership Renewal' AND Active_Membership__c = 'Active') OR (RecordType.Name = 'Membership Prospect' AND Web_Expire_Date__c != null))]);
    Map<Id,Opportunity> accIdwithOpp = new Map<Id,Opportunity>();
    
    for(Opportunity opp : oppList){
        if(!accIdwithOpp.containsKey(opp.AccountId)){
            accIdwithOpp.put(opp.AccountId,opp);
        }
    }
    
    for(Account acc : trigger.new){
        if(accIdwithOpp.containsKey(acc.id)){
            acc.Active_Membership_Opportunity__c = accIdwithOpp.get(acc.id).id;
        }
        else{
            acc.Active_Membership_Opportunity__c = null;
        }
    }
}
 
trigger act_opp_comms on Account (before insert, before update) {
    List<Opportunity> oppList = new List<Opportunity>([SELECT Id, Name,AccountId FROM Opportunity WHERE AccountID in : trigger.new 
AND ((RecordType.Name = 'Communications Council Renewal' AND Comms_Status__c = 'Active') OR (RecordType.Name = 'Communications Council Prospect' AND Web_Expire_Date__c != null))]);
    Map<Id,Opportunity> accIdwithOpp = new Map<Id,Opportunity>();
    
    for(Opportunity opp : oppList){
        if(!accIdwithOpp.containsKey(opp.AccountId)){
            accIdwithOpp.put(opp.AccountId,opp);
        }
    }
    
    for(Account acc : trigger.new){
        if(accIdwithOpp.containsKey(acc.id)){
            acc.Active_Comms_Opportunity__c = accIdwithOpp.get(acc.id).id;
        }
        else{
            acc.Active_Comms_Opportunity__c = null;
        }
    }
}

 
ClintLeeClintLee
Here is a nice write up by Kevin O'Hara on trigger frameworks.  The article discusses different approaches / best practices for handling trigger logic.

https://developer.salesforce.com/page/Trigger_Frameworks_and_Apex_Trigger_Best_Practices
ManojjenaManojjena
Hi Anthony,
I think your approach is wrong ,As your arequerying from Oportunity before insert of account .Can  u explain your requirment so that we can write trigger with best practice which will help you .

Thanks
Manoj
Abhishek BansalAbhishek Bansal
Hi Anthony,

I have merged your three triggers into one.
Please give a try with the below code :
 
trigger act_opp_subscrip on Account (before insert, before update) {
    List<Opportunity> oppList = new List<Opportunity>([SELECT Id, Name,AccountId FROM Opportunity WHERE AccountID in : trigger.new 
AND (((RecordType.Name = 'Subscription Renewal' AND Subscription_Status__c = 'Active') OR (RecordType.Name = 'Membership Renewal' AND Active_Membership__c = 'Active') OR(RecordType.Name = 'Communications Council Renewal' AND Comms_Status__c = 'Active') ) OR ((RecordType.Name = 'Subscription Prospect' OR RecordType.Name = 'Membership Prospect' OR RecordType.Name = 'Communications Council Prospect') AND Web_Expire_Date__c != null))]);
    Map<Id,Opportunity> accIdwithOpp = new Map<Id,Opportunity>();
    
    for(Opportunity opp : oppList){
        if(!accIdwithOpp.containsKey(opp.AccountId)){
            accIdwithOpp.put(opp.AccountId,opp);
        }
    }
    
    for(Account acc : trigger.new){
        if(accIdwithOpp.containsKey(acc.id)){
        	if(accIdwithOpp.get(acc.id).RecordType.Name == 'Subscription Renewal' || accIdwithOpp.get(acc.id).RecordType.Name == 'Subscription Prospect'){
        		acc.Active_Subscription_Opportunity__c = accIdwithOpp.get(acc.id).id;
        		acc.Active_Membership_Opportunity__c = null;
        		acc.Active_Comms_Opportunity__c = null;
        	}
        	else if(accIdwithOpp.get(acc.id).RecordType.Name == 'Membership Renewal' || accIdwithOpp.get(acc.id).RecordType.Name == 'Membership Prospect'){
        		acc.Active_Subscription_Opportunity__c = null;
        		acc.Active_Membership_Opportunity__c = accIdwithOpp.get(acc.id).id;
        		acc.Active_Comms_Opportunity__c = null;
        	}
            else if(accIdwithOpp.get(acc.id).RecordType.Name == 'Communications Council Renewal' || accIdwithOpp.get(acc.id).RecordType.Name == 'Communications Council Prospect'){
        		acc.Active_Subscription_Opportunity__c = null;
        		acc.Active_Membership_Opportunity__c = null;
        		acc.Active_Comms_Opportunity__c = accIdwithOpp.get(acc.id).id;
        	}
        }
    }
}

Please test your all cases with above trigger and let me know if you have you have any issues.

Thanks,
Abhishek
Anthony Zirilli 10Anthony Zirilli 10
Hi Abhishek,

I got thie error when running your code above: rror:Apex trigger act_opp_subscrip1 caused an unexpected exception, contact your administrator: act_opp_subscrip1: execution of BeforeUpdate caused by: System.SObjectException: SObject row was retrieved via SOQL without querying the requested field: Opportunity.RecordType: Trigger.act_opp_subscrip1: line 14, column 1

Any ideas?
Abhishek BansalAbhishek Bansal
Hi Anthony,

There was a field missing in query.
Now i have updated the code.
trigger act_opp_subscrip on Account (before insert, before update) {
    List<Opportunity> oppList = new List<Opportunity>([SELECT Id, Name,AccountId,RecordType.Name FROM Opportunity WHERE AccountID in : trigger.new 
AND (((RecordType.Name = 'Subscription Renewal' AND Subscription_Status__c = 'Active') OR (RecordType.Name = 'Membership Renewal' AND Active_Membership__c = 'Active') OR(RecordType.Name = 'Communications Council Renewal' AND Comms_Status__c = 'Active') ) OR ((RecordType.Name = 'Subscription Prospect' OR RecordType.Name = 'Membership Prospect' OR RecordType.Name = 'Communications Council Prospect') AND Web_Expire_Date__c != null))]);
    Map<Id,Opportunity> accIdwithOpp = new Map<Id,Opportunity>();
    
    for(Opportunity opp : oppList){
        if(!accIdwithOpp.containsKey(opp.AccountId)){
            accIdwithOpp.put(opp.AccountId,opp);
        }
    }
    
    for(Account acc : trigger.new){
        if(accIdwithOpp.containsKey(acc.id)){
        	if(accIdwithOpp.get(acc.id).RecordType.Name == 'Subscription Renewal' || accIdwithOpp.get(acc.id).RecordType.Name == 'Subscription Prospect'){
        		acc.Active_Subscription_Opportunity__c = accIdwithOpp.get(acc.id).id;
        		acc.Active_Membership_Opportunity__c = null;
        		acc.Active_Comms_Opportunity__c = null;
        	}
        	else if(accIdwithOpp.get(acc.id).RecordType.Name == 'Membership Renewal' || accIdwithOpp.get(acc.id).RecordType.Name == 'Membership Prospect'){
        		acc.Active_Subscription_Opportunity__c = null;
        		acc.Active_Membership_Opportunity__c = accIdwithOpp.get(acc.id).id;
        		acc.Active_Comms_Opportunity__c = null;
        	}
            else if(accIdwithOpp.get(acc.id).RecordType.Name == 'Communications Council Renewal' || accIdwithOpp.get(acc.id).RecordType.Name == 'Communications Council Prospect'){
        		acc.Active_Subscription_Opportunity__c = null;
        		acc.Active_Membership_Opportunity__c = null;
        		acc.Active_Comms_Opportunity__c = accIdwithOpp.get(acc.id).id;
        	}
        }
    }
}
Let me know if you still have any issues.

Regards,
Abhishek
 
Anthony Zirilli 10Anthony Zirilli 10
It seems to only update the Active_Subscription_Opportunity__c field and nothing else. 

Any ideas?
Abhishek BansalAbhishek Bansal
As per my understanding the above code is fine and it should update all three fields according to the Record Type.
So in order to resolve your current issue i have to debug the code in your ORG.
So if it is possible for you than please share your credentials or you can contact me at :
Gmail : abhishek.bansal@metacube.com
SkpeId : abhishek.bansal2790

It would be easy to help you out there.

Thanks,
Abhishek
Amit Chaudhary 8Amit Chaudhary 8
Please check below post. for same issue
http://amitsalesforce.blogspot.in/2015/06/trigger-best-practices-sample-trigger.html

1) One Trigger Per Object
A single Apex Trigger is all you need for one particular object. If you develop multiple Triggers for a single object, you have no way of controlling the order of execution if those Triggers can run in the same contexts

2) Logic-less Triggers
If you write methods in your Triggers, those can’t be exposed for test purposes. You also can’t expose logic to be re-used anywhere else in your org.

3) Context-Specific Handler Methods
Create context-specific handler methods in Trigger handlers

4) Bulkify your Code
Bulkifying Apex code refers to the concept of making sure the code properly handles more than one record at a time.

5) Avoid SOQL Queries or DML statements inside FOR Loops
An individual Apex request gets a maximum of 100 SOQL queries before exceeding that governor limit. So if this trigger is invoked by a batch of more than 100 Account records, the governor limit will throw a runtime exception

6) Using Collections, Streamlining Queries, and Efficient For Loops
It is important to use Apex Collections to efficiently query data and store the data in memory. A combination of using collections and streamlining SOQL queries can substantially help writing efficient Apex code and avoid governor limits

7) Querying Large Data Sets
The total number of records that can be returned by SOQL queries in a request is 50,000. If returning a large set of queries causes you to exceed your heap limit, then a SOQL query for loop must be used instead. It can process multiple batches of records through the use of internal calls to query and queryMore

Please let us know if this will help u

Thanks
Amit Chaudhary