4

I am working through my first implementation of a trigger + handler framework. I'm trying to figure out what I think is a pretty basic scenario: how to remove items from my trigger so that only those that meet certain criteria are processed, without losing the reference to the original records (and therefore needing to do a separate DML, rather than just update the records' values in the before trigger context)

This is my understanding of how this works, please stop me right here if I'm on the wrong track!

I've read about trigger frameworks in a couple of places, namely Kevin Ohara. It's a bit over my head but I think I get the basic idea.

My understanding is that trigger should call a handler class that in turn calls the classes that actually do stuff. This allows us to keep the trigger simple, the handler controls the order, and the actual work happens in business logic classes.

This is working fine for me in the before insert context where I'm just passing the records from the trigger to the handler and on to the class without doing anything to them (ie trying to remove records from being processed). The part I'm stuck on is in the before update context. Here, I only want to process records that are changing in a way that is relevant to my business logic.

So, in my handler, I'm trying to go through the records that were passed in from the trigger and only process the ones where the country or email has changed.

My understanding is that if I create a new list with the records I want to process, I lose the reference to trigger.new and so I need to update the records with a DML statement which creates new problems. So, instead, I thought I could iterate through the list, determine which ones need to be processed, and pass those along to my business class.

I'm looking for help validating this fundamental approach (using a trigger handler to remove items that shouldn't be processed) and getting pointed in the right direction of how to actually do it because I feel like I'm struggling with something that probably has a straightforward solution!

Here's what I've got so far:

Lead Trigger

  trigger leadTrigger on Lead (
    
  before insert, after insert, 
  before update, after update, 
  before delete, after delete) {

  if (Trigger.isBefore) {
    if (Trigger.isInsert) {
      LeadTriggerHandler handler = new LeadTriggerHandler(Trigger.New);
      handler.beforeInsert();
        
    } 
    if (Trigger.isUpdate) {

      LeadTriggerHandler handler = new LeadTriggerHandler(Trigger.OldMap, Trigger.NewMap, Trigger.New);
      handler.beforeUpdate();
        
    }

Lead Handler

public class LeadTriggerHandler {
    Map<Id, Lead> oldLeadsMap;
    Map<Id, Lead> newLeadsMap;
    List<Lead> oldLeadsList;
    List<Lead> newLeadsList;
    
    public LeadTriggerHandler (
        List<Lead> newTriggerLeadsList) {
            newLeadsList = newTriggerLeadsList;
        } 
    
    public LeadTriggerHandler (
      Map<Id, Lead> oldTriggerLeadsMap,
      Map<Id, Lead> newTriggerLeadsMap,
      List<Lead> newTriggerLeadsList) {
            oldLeadsMap = oldTriggerLeadsMap;
            newLeadsMap = newTriggerLeadsMap;
            newLeadsList = newTriggerLeadsList;
        }  
    
    public void beforeInsert() {
        
         gdprGeography.leadGDPR(newLeadsList);
        
    }
    
    public void beforeUpdate () {
        newLeadsList.clear();

        for (lead newLead : newLeadsMap.values()) {
            lead oldLead = oldLeadsMap.get(newLead.Id);
            string oldLeadCountry = oldLead.Country;
            string newLeadCountry = newLead.Country;
            string oldLeadEmail = oldLead.Email;
            string newLeadEmail = newLead.Email;
            
            if (oldLeadCountry == newLeadCountry ||
               oldLeadEmail == newLeadEmail) {
                newLeadsMap.remove(newLead.Id);
            }
        }
        
        for (lead mapLead : newLeadsMap.values()){
            newLeadsList.add(mapLead);
        }
        
        gdprGeography.leadGDPR(newLeadsList);
        
    }
}

Business logic class

My class's method takes a list of leads and then uses custom metadata types to get information about the persons country's data privacy requirements. This part is working fine.

public class gdprGeography {    
    
    /* contains methods for retrieving custom metadata for data privacy
     * including whether a lead/contact is covered by GDPR, based on their address,
     * and what their country requires for unsolicited marketing
    */  
    
    public static void leadGDPR(list<lead> leadsToProcess) {
//do stuff!
}
}

2 Answers 2

14

Background: understanding collections and references

Your understanding isn't quite correct.

Collections (lists, sets, and maps) generally store non-primitive types (i.e. pretty much everything that isn't String, Integer, Boolean, etc...) as references. This behavior holds until you explicitly break/replace the reference with a new one, typically either by calling new or using clone().

To illustrate this, try running the following anonymous apex

Account acct1 = new Account(), acct2 = new Account();
List<Account> acctList1 = new List<Account>{acct1, acct2};
List<Account> acctList2 = acctList1;
List<Account> acctList3 = acctList1.clone();
List<Account> acctList4 = new List<Account>{acct1, acct2};
List<Account> acctList5 = acctList1.deepClone();

acctList3.remove(0);

Integer i = 0;
for(Account acct :acctList4){
    // When changing data on one object instance reference, it gets changed on ALL
    //   references to that object instance reference (and on the original instance)
    acct.Name = 'name' + i;
    i++;
}

// This also happens when you work with the original instance
acct1.Description = 'words';

system.debug(acct1);      // Account:{Name=name0, Description=words}
system.debug(acct2);      // Account:{Name=name1}
system.debug(acctList1);  // (Account:{Name=name0, Description=words}, Account:{Name=name1})
system.debug(acctList2);  // (Account:{Name=name0, Description=words}, Account:{Name=name1})
system.debug(acctList3);  // (Account:{Name=name1})
system.debug(acctList4);  // (Account:{Name=name0, Description=words}, Account:{Name=name1})

// Using .deepClone() "breaks" the reference, and gives you a completely independent copy
// .deepClone() is available on collections, for SObjects you'd just use .clone()
system.debug(acctList5);  // (Account:{}, Account:{})

When it comes to adding and removing items from collections, it might help if you visualize references like water. Changes only flow downhill.

Since we set acctList2 = acctList1, the two lists are effectively on the same level. Adding or removing records from one affects the other.
If you clone a list (as we do with acctList3) or make a new list (with the same object instance references inside), adding or removing records from those do not affect the 'upstream' lists (acctList1 and acctList2).

Passing a collection into your handler is like direct assignment (acctList2 = acctList1).

One of the special things about trigger.old, trigger.new, trigger.oldMap, and trigger.newMap is that we can't add or remove items from those collections (and trigger.old/oldMap are read-only). You'll get an error if you try this on a collection that hasn't been cloned (acctList3 = new List<Account>(acctList1); also has the same effect as using .clone()).

There's just one more piece of advice before I move on. It's generally not recommended to add or remove records from the same collection that you're iterating over. It might work in some cases, you might get a compile-time or run-time error in other cases. It's just better to avoid potential issues.

How do I keep a portion of a collection to actually perform work on?

There are 2 main approaches that I find myself using:

  1. Iterate over one collection, and add target records to a second collection
  2. Iterate over a map, add the key for the target map record to a set, then clone() and removeAll() (or retainAll())

The first approach pretty much works for anything.

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

for(Lead l :myReferenceToTriggerNew){
    Lead oldLead = myReferenceToTriggerOldMap.get(l.Id);

    if(oldLead.Some_Field__c != l.Some_Field__c){
        // 'l' still maintains its reference to the instance in trigger.new
        leadsToProcess.add(l);
    }
}

otherWork(leadsToProcess);

The second approach is mainly useful for maps, but can work on sets as well

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

for(Lead l :myReferenceToTriggerNew){
    Lead oldLead = myReferenceToTriggerOldMap.get(l.Id);

    if(oldLead.Some_Field__c != l.Some_Field__c){
        idsToProcess.add(l.Id);
    }
}

Map<Id, Lead> leadsToProcess = myReferenceToTriggerNew.clone();
// The Map class doesn't give us anything to remove multiple items at a time, but
//   the Set class does.
// By removing items from a Map's keySet, we also remove the items from the map.
leadsToProcess.keySet().retainAll(idsToProcess);
otherWork(leadsToProcess.values());

There's probably some use case where the second approach makes sense and/or has an advantage over the first approach, I just can't remember an example.

In most cases, you'll likely want to use the first approach (just building a new collection that only contains the data you want to work on).

Side note

If you have a list to add to another list, you can use .addAll().

So instead of this

for (lead mapLead : newLeadsMap.values()){
    newLeadsList.add(mapLead);
}

you could use this

newLeadsList.addAll(newLeadsMap.values());
1
  • I appreciate the detailed answer and that you threw in a few free tips to boot! I had a feeling I was missing something fundamental. Thanks for getting me pointed in the right direction.
    – aljmet
    Feb 7 at 17:14
1

You do not actually need to pick up only records that suit conditions in your handler. Think about it this way: why should you pass list of records to business logic class instead of only one separate record?

Example:

Business logic class:

public class gdprGeography {
    public static void leadGDPR(Lead toChange) {
        //do stuff with Lead record
        //for example
        toChange.Phone = '123';
    }
}

Lead Trigger Handler class:

//...
for (Lead newLead : newLeadsList) {
    Lead oldLead = oldLeadsMap.get(newLead.Id);
    if (/*some condition here to CHANGE lead*/) {
        gdprGeography.leadGDPR(newLead);
    }
}
//...
2
  • @ Mariia, If I had hundreds or thousands of records, would repeatedly calling a method like this be a problem? My understanding is to make all my code work with lists (bulkify!) and I'm curious if this is a context where that approach is unnecessary.
    – aljmet
    Feb 7 at 17:18
  • This is even more optimized approach, because you will not run second for-loop on selected records, you'll do everything in the first one. Feb 7 at 19:35

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.