[SalesForce] Need help in the 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 trigger.new to the class as a list, however, this (I think) prevented me from looping over trigger.new 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 trigger.new 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 trigger.new, 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!

Thanks!

NEW CODE: Thanks for the help so far!

Trigger:

trigger transferRelationshipTeamFromAcct on Financial_Account__c (before insert, before update) {
    FinancialAccountManager.handleUpdateTransferRelationshipTeamFromAcct(trigger.new);
}

Class:

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 :trigger.new
    ]){
        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;
    }
}
}

Best Answer

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 ;
    }

  }

Related Solutions

[SalesForce] Checking ‘Correct’ Previous value method in Trigger design best practice

After going over this question rather thoroughly, I think I came up with a utility that should do the trick. It's basically what @greenstork suggested, just implemented a little differently.

Below is the utility code along with implementation close to the original example:

public with sharing class BucketingUtility 
{
    public interface ITriggerComparisonFilter
    {
        Set<sObject> FilterResults(List<sObject> newsObjectList, Map<Id, sObject> oldMapForsObjects);
    }

    //Simple marker interface
    public interface IFilterable { }

    //Should be in in the IsWonOpportunityFilter class, but since I wrote it in one file, it is outside of it.
    public static Map<Id, Boolean> OldOpportunitiesThatWon = new Map<Id, Boolean>();

    public class IsWonOpportunityFilter implements ITriggerComparisonFilter, IFilterable
    {
        public Set<sObject> FilterResults(List<sObject> newsObjectList, Map<Id, sObject> oldMapForsObjects)
        {
            List<Opportunity> newOpportunities = (List<Opportunity>)newsObjectList;
            Map<Id, Opportunity> oldOpportunityMap = (Map<Id, Opportunity>)oldMapForsObjects;

            Set<sObject> wonOpportunities = new Set<sObject>();

            for(Opportunity singleOpportunity : newOpportunities)
            {
                Boolean oldHasWon = (OldOpportunitiesThatWon.containsKey(singleOpportunity.Id)) ? 
                    OldOpportunitiesThatWon.get(singleOpportunity.Id) : oldOpportunityMap.get(singleOpportunity.Id).isWon;

                if(singleOpportunity.isWon && !oldHasWon)
                    wonOpportunities.add(singleOpportunity);

                if(oldHasWon != singleOpportunity.isWon)
                    OldOpportunitiesThatWon.put(singleOpportunity.Id,singleOpportunity.isWon);
            }

            return wonOpportunities;
        }
    }

    public enum Bucket
    {
        OpportunitiesThatWon
    }

    private static Map<Bucket, IFilterable> BucketMethods = 
        new Map<Bucket, IFilterable>{ Bucket.OpportunitiesThatWon => new IsWonOpportunityFilter() };   

    private static Map<Bucket, Set<sObject>> BucketResults = new Map<Bucket, Set<sObject>>();

    private static Set<sObject> ProviderTriggerFilterResults(Bucket methodToBucketBy, List<sObject> newList, Map<Id, sObject> oldMap)
    {
        //I'm assuming the bucket will exist in BucketMethods
        ITriggerComparisonFilter triggerFilterMethod = (ITriggerComparisonFilter)BucketMethods.get(methodToBucketBy);
        BucketResults.put(methodToBucketBy, triggerFilterMethod.FilterResults(newList, oldMap));
        return BucketResults.get(methodToBucketBy);
    }

    public static Set<sObject> GetTriggerFilterResults(Bucket methodToBucketBy, List<sObject> newList, Map<Id, sObject> oldMap)
    {
        if(!BucketResults.containsKey(methodToBucketBy))
            return ProviderTriggerFilterResults(methodToBucketBy, newList, oldMap);
        if(BucketResults.get(methodToBucketBy) == null)
            return ProviderTriggerFilterResults(methodToBucketBy, newList, oldMap);
        return BucketResults.get(methodToBucketBy);
    }

    public static void ClearFilteredResults(Bucket bucketMethodToClearResults)
    {
        BucketResults.put(bucketMethodToClearResults, null);
    }

    public static Map<Id, sObject> GetTriggerFilterResultMap(Bucket methodToBucketBy, List<sObject> newList, Map<Id, sObject> oldMap)
    {
        return new Map<Id, SObject>(new List<sObject>(GetTriggerFilterResults(methodToBucketBy, newList, oldMap)));
    }

    public static Set<Id> GetTriggerFilterResultIds(Bucket methodToBucketBy, List<sObject> newList, Map<Id, sObject> oldMap)
    {
        return GetTriggerFilterResultMap(methodToBucketBy, newList, oldMap).keySet();
    }
}

Now you can share your results with however many methods within your particular trigger handler. Here is an example of how to use it:

public with sharing class OpportunityHandler 
{
    public static void OpportunityAfterUpdate(List<Opportunity> newOpportunities, List<Opportunity> oldOpportunities, Map<Id,Opportunity> newMap, 
        Map<Id,Opportunity> oldMap)
    {
        ExampleMethodForDemonstration(newOpportunities, oldOpportunities, newMap, oldMap);
        ExampleMethodIIForDemonstration(newOpportunities, oldOpportunities, newMap, oldMap);
        BucketingUtility.ClearFilteredResults(BucketingUtility.Bucket.OpportunitiesThatWon);
    }

    private static void ExampleMethodForDemonstration(List<Opportunity> newOpportunities, List<Opportunity> oldOpportunities, 
        Map<Id, Opportunity> newMap, Map<Id, Opportunity> oldMap)
    {
        Set<Id> opportunityIdsForWonSet = BucketingUtility.GetTriggerFilterResultIds(BucketingUtility.Bucket.OpportunitiesThatWon, 
            newOpportunities, oldMap);

        //Operate on won Opportunities Won Id Set in Example Method
    }

    private static void ExampleMethodIIForDemonstration(List<Opportunity> newOpportunities, List<Opportunity> oldOpportunities, 
        Map<Id, Opportunity> newMap, Map<Id, Opportunity> oldMap)
    {
        Set<Id> opportunityIdsForWonSet = BucketingUtility.GetTriggerFilterResultIds(BucketingUtility.Bucket.OpportunitiesThatWon, 
            newOpportunities, oldMap);

        //Operate on won Opportunities Won Id Set in Example Method II
    }
}

As you can see, I can now share the Id Set of all won Opportunities between method calls inside of the OpportunityHandler's OpportunityAfterUpdate method. I also only retrieve the Id Set once per call to OpportunityAfterUpdate. Once OpportunityAfterUpdate's methods are all complete, I clear the results using the BucketingUtility.ClearFilteredResults. This specifies that we should recalculate, using our filter to retrieve our Ids again, if another call occurs - like a workflow rule kicks off the trigger again. We still have the original static map to check against recursion still in place and should still guard against that.

[SalesForce] Updating a field with a value from parent during insert trigger

Actually you don't need to do that first insert to get the object's id. You need the id of the parent to do the query, and you should have that since it should be populated in the look-up field. I would do something like this:

// use a before insert trigger
// loop through trigger new, putting parent ids into a set<Id>
// query for parent objects, then use that list as argument in map constructor
// loop through trigger new again, get parent object from map, set field value

Update

I wanted to add a working example here to better illustrate my answer above. Note that if you are working with a look-up relationship, you should check to make sure that there actually is a parent record before trying to get a value from it.

trigger Contact on Contact (before insert){

    if (trigger.isBefore){
        if (trigger.isInsert){
            SetContactField.setAccountName(trigger.new);
        }
    }

}

public class SetContactField{

    public static void setAccountName(list<Contact> TriggerNew){

        list<Id> AccountIds = new list<Id>();

        for (Contact c : TriggerNew){
            AccountIds.add(c.AccountId);
        }

        map<Id,Account> RelatedAccounts = new map<Id,Account>([SELECT Id, Name FROM Account WHERE Id IN :AccountIds]);

        for (Contact c : TriggerNew){
            Account RelatedAccount = RelatedAccounts.get(c.AccountId);
            if (RelatedAccount == NULL){  // In a master detail relationship, this record is guaranteed to exist. 
                c.AccountName__c = NULL;
            } else {
                c.AccountName__c = RelatedAccount.Name;
            }
        }

    }

}

Best Answer

Related Solutions

[SalesForce] Checking ‘Correct’ Previous value method in Trigger design best practice

[SalesForce] Updating a field with a value from parent during insert trigger

Related Topic