Find your content:

Search form

You are here

Need help in my attempt to follow a 'best practice' for writing triggers


I have read that it is a considered a best practice when writing a trigger to have as little code as possible in the trigger itself, and to instead call a class from the trigger. I wrote a trigger that did this, and as an 'After' trigger, it worked, however, I ran into an issue when I realized that it should actually be a 'Before' trigger.

I followed a template that suggested passing to the class as a list, however, this (I think) prevented me from looping over in the class when making my field updates, so the field changes weren't being committed to the database. Is there a way to loop over a list containing the same sObjects as and have the changes commit to the database?

I have since abandoned the best practice and brought all of the code directly into the trigger's body, looping over, and it works as it should, but, as a new developer, I would like to stick to best practices when possible, so any incite would be appreciated!


NEW CODE: Thanks for the help so far!


trigger transferRelationshipTeamFromAcct on Financial_Account__c (before insert, before update) {


public class FinancialAccountManager {
public static void handleUpdateTransferRelationshipTeamFromAcct(List<Financial_Account__c> FinAcctsNewMap){
    Map<Id, Financial_Account__c> mapFinAccts = new Map<Id, Financial_Account__c>();

    for(Financial_Account__c finAcct : [
        SELECT Id, 
            Wealth_Advisor_Lookup__c, Portfolio_Manager_Lookup__c, Trust_Officer_Lookup__c,
            Other_Team_Member_1_Lookup__c, Other_Team_Member_2_Lookup__c,
            client__c, client__r.Wealth_Advisor__c, client__r.Portfolio_Manager_Lookup__c,
            client__r.Trust_Officer__c, client__r.Other_Team_Member_1__c, client__r.Other_Team_Member_2__c
        FROM Financial_Account__c
        WHERE Id IN
        mapFinAccts.put(finAcct.Id, finAcct);

    for(Financial_Account__c finAcct : FinAcctsNewMap){
        finAcct.Wealth_Advisor_Lookup__c = mapFinAccts.get(finAcct.Id).client__r.Wealth_Advisor__c;
        finAcct.Portfolio_Manager_Lookup__c = mapFinAccts.get(finAcct.Id).client__r.Portfolio_Manager_Lookup__c;
        finAcct.Trust_Officer_Lookup__c = mapFinAccts.get(finAcct.Id).client__r.Trust_Officer__c;
        finAcct.Other_Team_Member_1_Lookup__c = mapFinAccts.get(finAcct.Id).client__r.Other_Team_Member_1__c;
        finAcct.Other_Team_Member_2_Lookup__c = mapFinAccts.get(finAcct.Id).client__r.Other_Team_Member_2__c;

Attribution to: jackerman09

Possible Suggestion/Solution #1

Trigger.New is basically a list of object on which we write trigger.

You can loop through the Trigger.New easily if you pass as a parameter to the class


trigger SFA_OnSampleDropTrigger on Sample_Drop__c (before insert,after insert,before update) {
system.debug('Inside SFA_OnSampleDropTrigger');

if(Trigger.isAfter && Trigger.isInsert){    
    system.debug('Commencing after insert processing');
    //After a sample drop record has been inserted it should trigger a sample inventory recalculation


Now as you will see in my class i will have a list of object as parameter

public with sharing class SFA_OnSampleDropTriggerHelper {

//@Name: extractSampleInventoryLine
//@Description: Fiinds out the Sample Inventory Line record related to each Sample Drop record created and then calls 
//the inventory recalculation functionality
//@Input: List<Sample_Drop__c>
//@Return: void
//@Created: 2-AUG-12
public static void extractSampleInventoryLine(List<Sample_Drop__c> lstDropped){
    system.debug('Start of extractSampleInventoryLine');
    if(lstDropped==null || lstDropped.size()==0){
        throw new SFA_ApplicationException('input parameter list is empty');
    system.debug('Number of Sample Drop records being processed: '+lstDropped.size());
    Map<String, Integer> mapSilIdToQty=new Map<String, Integer>();//Map of sample inventory line id to the quantity involved in the sample drop
    Map<String, String> mapSilIdToSDId=new Map<String, String>();//Map of sample inventory line id to the sample drop id

    for(Sample_Drop__c s:lstDropped){
        mapSilIdToQty.put(s.Product__c, Integer.valueOf(SFA_Utility.checkNull(s.Quantity__c)));

    system.debug('mapSilIdToQty.size(): '+mapSilIdToQty.size());
    system.debug('mapSilIdToSDIdx.size(): '+mapSilIdToSDId.size());

    //Invoking inventory recalculation function

    system.debug('End of extractSampleInventoryLine');

Attribution to: Mohith Shrivastava

Possible Suggestion/Solution #2

Yes it is generally considered best practice to delegate processing to handler classes. In a before trigger you don't need an explicit update to persist your changes as long as they are on the sObject records that set off the trigger.

If you are making changes on the triggering records the thumbrule is to use a before trigger.

If however as a consequence of a update or insert of certain records you want to do an associated task, such as creating or updating related records, use an after trigger.

    Trigger AccountBefore on Account (before insert, before update)
AccountHandler accHandler = new AccountHandler():

Public with sharing class AccountHandler(){
Public void processRecords(List<Account> accts){
For (Account acc : accts){
Acc.field = value;

Attribution to: techtrekker

Possible Suggestion/Solution #3

Now that you've posted code I see the issue here: you're re-querying the records into a new variable and then making changes to those records.

This answer assumes you're working with a before update trigger, some of this advice does not apply to before insert triggers as you cannot query the sObjects in that case, and a map with record IDs as keys is also not viable.

Realize that the sObject in are special: any changes you make to them in before triggers are automatically committed to the database as you're trying to take advantage of. However this special property only applies to the specific instances of them in; if you create a new list of sObjects from a query this same magic does not apply.

A common approach here would be to put your query results into a Map<Id,Financial_Account__c> data structure. You then want to change for(Financial_Account__c finAcct : FinAcctsNewMapWithQuery) to iterate over the sObjects passed in from (FinAcctsNewMap). You can then pull in the sObject you queried (that has all the related fields populated) from the map you created earlier.

Attribution to: ca_peterson

Possible Suggestion/Solution #4

Here you go - this should work for both Inserts and Updates

public class FinancialAccountManager {
public static void handleTransferRelationshipTeamFromAcct(List<Financial_Account__c> FinAcctsNewMap){

Set<Id> clients = new Set<Id>{};

for(Financial_Account__c finAcct : finAcctsNewMap){
clients.add(finAcct.Client__c); //collect all the client Ids we are interested in


//Query the client attributes
    Map <Id, Client__c> clientsMap = new Map<Id, Client__c)([
        SELECT Id, Wealth_Advisor__c, Portfolio_Manager_Lookup__c,Trust_Officer__c, Other_Team_Member_1__c, Other_Team_Member_2__c
        FROM Client__c
        WHERE Id IN :clients]);

    for(Financial_Account__c finAcct : FinAcctsNewMap){

           finAcct.Portfolio_Manager_Lookup__c = clientsMap.get(finAcct.Client__c).Portfolio_Manager_Lookup__c;

            finAcct.Trust_Officer_Lookup__c = clientsMap.get(finAcct.Client__c).Trust_Officer_Lookup__c ;

            finAcct.Other_Team_Member_1_Lookup__c = clientsMap.get(finAcct.Client__c).Other_Team_Member_1_Lookup__c ;

            finAcct.Other_Team_Member_2_Lookup__c = clientsMap.get(finAcct.Client__c).Other_Team_Member_2_Lookup__c ;


Attribution to: techtrekker
This content is remixed from stackoverflow or stackexchange. Please visit

My Block Status

My Block Content