2

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;
    }
}
}
  • 1
    Can you post the failing code? It might help point out what caused the issue. – ca_peterson Oct 8 '12 at 19:28
  • Sure, added, it's a little long. – jackerman09 Oct 8 '12 at 19:41
2

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

  }
  • Thanks, I'm going to try this, but would you mind explaining why this works but what I have doesn't? I'm trying to learn as I code, so I am doing my best to have an understanding of all of the changes that I make. Thanks again. – jackerman09 Oct 8 '12 at 20:19
  • Sure, what you're trying to do is when inserting/ updating Financial Accounts, whether the Trust Officer, Portfolio Manager, etc set on the Financial Account is the same as the ones set on the Client. Client I'm assuming is a reference table that already exists. So what we're doing is collecting all the client ids, querying client attributes, and then comparing whether the attributes on the Financial Accounts match those with the ones et on the client. – techtrekker Oct 8 '12 at 20:21
  • Client is a record that is the parent in a master-detail relationship with Financial Account. I'm looking at your code, and it says here (boards.developerforce.com/t5/Visualforce-Development/…) that populating a map directly from an SOQL query is not possible without iterating through it. Has this changed since that post was made? – jackerman09 Oct 8 '12 at 20:23
  • 1
    You can create a map of <Id, sObject> in the way I've used - you cant use any other field as the key using this method, just Id. – techtrekker Oct 8 '12 at 20:24
  • Gotcha, thanks. What is the second map, 'mapFinAccts' doing? I don't see it get populated or used at all? – jackerman09 Oct 8 '12 at 20:29
2

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 Trigger.new 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 Trigger.new; 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 Trigger.new (FinAcctsNewMap). You can then pull in the sObject you queried (that has all the related fields populated) from the map you created earlier.

  • Thanks! The trigger I am writing is both a Before Update and Before Insert. Without using the Map technique, how can I address the fields when Inserting a Financial Account? – jackerman09 Oct 8 '12 at 19:56
  • Thanks for the help so far! I've updated my post to reflect the changes I've made to the code. I've gone back to the trigger/class best practice (with your guidance), and it is working for Updates, however, as you suggested it would, it is getting screwed up by the Inserts. How can I handle them in this context? Thanks! – jackerman09 Oct 8 '12 at 20:10
  • You're really going to want to use multiple Map data structure to hold the info about the related records, rather than re-querying your objects with those relationships included. Think a Map<Id,Account>, a Map<Id,Contact> etc for each type of related record. I'll try and update my answer with more detail if I can find some time. – ca_peterson Oct 8 '12 at 20:23
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

Eg:

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
    SFA_OnSampleDropTriggerHelper.extractSampleInventoryLine(trigger.new);
}

}

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)));
        mapSilIdToSDId.put(s.Product__c,s.Id);
    }


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

    //Invoking inventory recalculation function
    SFA_SamplingManager.recalculateInventory(mapSilIdToQty,mapSilIdToSDId);


    system.debug('End of extractSampleInventoryLine');
    }
  }
  • In your example you used an After trigger, which I was using at first, and it worked for me. However, when I changed it to a Before trigger and was looping over the equivalent of your 1stDropped list, the changes were not being made to the database. Would your code still work in a Before scenario? – jackerman09 Oct 8 '12 at 19:37
  • Ya the trigger should work in before insert case as well .Can you explain further what did not work for You?thanks – Mohith Shrivastava Oct 8 '12 at 19:40
  • I put in some system.debugs and it shows that the changes are being made, but when I look at the record the changes are not there. Based on ca_peterson's comments below it sounds like I created a separate instance of the sObjects that I was then editing, which is why there were not being committed to the database. Thanks! – jackerman09 Oct 8 '12 at 19:57
  • @jackerman09 i totally agree with ca_peterson and its correct that you had different instance hence voted him up – Mohith Shrivastava Oct 8 '12 at 20:00
1

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():
AccHandler.processRecords(trigger.new);
}

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

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.