0

Is there any performance benefit of excluding records in logic directly in the trigger to avoid instantiating another class (TriggerHandler) that would essentially do the same thing? I know best practice is option 2 (which also provides the benefit of being cleaner and having a separation of concern). This is more strictly performance speaking.

Example: If there's a trigger on user but I want to exclude lots of created users for community

Option 1:

trigger UserMasterTrigger on User (after insert) {
    if(Trigger.isAfter){
        if(Trigger.isInsert){
            for(User userRec: Trigger.new){
                if(String.isBlank(userRec.Portal_User_Status__c)){
                    internalUsers.add(userRec);
                }
            }
            UserTriggerHandler userHandler = new UserTriggerHandler();
            userHandler.onAfterInsert(internalUsers);

        }
     }
   //pass only internal users to handler. Only creates/passes to handler if there's 
   records to pass
}

Handler

public class UserTriggerHandler {
//can start business logic immediately
}

versus Option 2

trigger UserMasterTrigger on User (after insert) {
    UserTriggerHandler userHandler = new UserTriggerHandler();
    if(Trigger.isAfter){
        if(Trigger.isInsert){
            userHandler.onAfterInsert(trigger.new);
        }
     }
}

Handler

public class UserTriggerHandler {
        //does the logic to exclude in handler, always calls handler regardless
        onAfterInsert(List<User> userList){
        List<User> internalUsers = new List<User>();
        for(User userRec : userList){
            if(String.isBlank(userRec.Portal_User_Status__c)){
                internalUsers.add(userRec);
            }
        }
        //do logic on internal user list in different class or same.
    }
}
1
  • 1
    I personally like the fflib (Enterprise Patterns) framework/pattern - check it out in TrailHead
    – cropredy
    Dec 10 '19 at 23:27
5

Performance will not be significantly affected by either approach. However, I think neither is really best practice.

My main argument against filtering in the trigger is that is difficult to test well, and involves a lot of unnecessary database interaction in your tests to achieve proper coverage. That can add up to significant increases in how much of your life you spend waiting for unit tests to run. Don't do that to yourself or anyone else where you can avoid it. Another note about why you should avoid this pattern of filtering is that for a given event, you might have multiple separate filters, so it would not be appropriate to use in any such case.

Your second approach is closer to best practice, but it still muddies up your handler impelementation and has the handler take on far too much responsibilty, in my opinion. As written, even for a trigger event with only one filter and one action, you have two distinct operations in one method (filter and action), rather than writing two separate methods. This approach will not scale.

Real time performance should be more or less equal across the two approaches you posit and that which I recommend below, but the below pattern should yield better cyclomatic coverage, a better organized and easier to write test suite, and significantly lower test run times overall.


Trigger

Delegate only.

trigger MyObject on MyObject__c (...)
{
    MyObjectTriggerHandler handle = new MyObjectTriggerHandler(trigger.new, trigger.oldMap);

    switch on trigger.operationType
    {
        when AFTER_INSERT { handle.afterInsert(); }
    }
}

Handler

Compose filters and actions.

public with sharing class MyObjectTriggerHandler
{
    @TestVisible static Boolean bypassTrigger = false;
    // this flag is very handy for unit test run times

    final List<MyObject__c> newRecords;
    final Map<Id, MyObject__c> oldMap;
    public MyObjectTriggerHandler(List<MyObject__c> newRecords, Map<Id, MyObject__c> oldMap)
    {
        this.newRecords = newRecords;
        this.oldMap = oldMap;
    }

    public void afterInsert()
    {
        if (bypassTrigger) return;

        MyObjectService.doStuff(
            MyObjectService.shouldDoStuff(newRecords)
        );
        // this approach means your handler will scale
        // and be easy to read and understand even if
        // you perform many operations in one event
    }
}

Service(s)

Within your service, split your filters and actions into separate methods. It may well be worth a separate filters class.

public with sharing class MyObjectService
{
    public static List<MyObject__c> shouldDoStuff(List<MyObject__c> records)
    {
        List<MyObject__c> output = new List<MyObject__c>();
        for (MyObject__c record : records)
        {
            if (/*do your filtering*/)
            {
                output.add(record)
            }
        }
        return output;
    }
    public static void doStuff(List<MyObject__c> records)
    {
        // only the action here, process all records
    }
}
0
1

Without measuring: I wouldn't expect any performance differences between the two options. Both basically contain the same statements, just in different order. The only difference really is the size of the list passed from trigger to trigger handler method and as this does not create a copy of the list. (see https://developer.salesforce.com/blogs/developer-relations/2012/05/passing-parameters-by-reference-and-by-value-in-apex.html why Salesforce calling this call-by-reference)

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.