10

For a generic trigger, are there circumstances where one of the following patterns is better than the other?

trigger ObjectTrigger on Object (after event) {
  ObjectTriggerHandler oth = new ObjectTriggerHandler();
  if ((Trigger.isAfter) && (Trigger.isEvent)) {
    oth.onAfterEvent(Trigger.new); // non-static method
  }
}

vs

trigger ObjectTrigger on Object (after event) {
  if ((Trigger.isAfter) && (Trigger.isEvent)) {
    ObjectTriggerHandler.onAfterEvent(Trigger.new); // static method
  }
}

This question has been asked at least twice on dev.SF forum without a clear answer.

  • 1
    Does it have to be a single class? The clear winner in terms of reducing boilerplate code and consistency is to use a TriggerHandler base class (e.g. github.com/kevinohara80/sfdc-trigger-framework) and have your individual trigger handlers inherit from it, but that's been answered in so many places I don't know if you've already seen that pattern and dismissed it. – IllusiveBrian Dec 13 '19 at 22:15
  • I had not seen that yet, thanks for the tip. – Foo Bar Dec 20 '19 at 13:06
8

We use the object oriented approach of using non-static methods because of the massive benefits we can derive in unit testing (and potentially also in production cases, though we don't do that yet).

We take a "dependency injection factory method" or "dependency injection singleton access" approach. Each of our trigger handlers provides an interface or set of virtual methods, meaning we can provide handlers that vary the behaviour. When the trigger wants to access an instance it uses a factory method (if parameters are required for the object initialization) or a singleton access method. Here I'll provide the singleton example for simplicity.

The factory method/singleton access is itself static. Behind it we leverage a Type. The Type is held in a private static value. Let's start with an example:

public virtual with sharing class MyHandler {
    private static Type instanceType = MyHandler.class;
    private static MyHandler instance = null;

    public static Type setInstanceType(Type instanceType) {
        Type originalType = MyHandler.instanceType;

        if (instanceType != null) {
            MyHandler.instanceType = instanceType;
            instance = null;
        }

        return originalType;
    }

    public static MyHandler getInstance() {
        if (instance == null) {
            instance = (MyHandler) instanceType.newInstance();
        }

        return instance;
    }

    public virtual void handleInsert(List<MyObject__c> newObjects) {
        ...;
    }
}

Something to note is the setInstanceType method. This lets us change the implementation used whenever we want to, while instanceType defaults to the actual "production" type so we don't have to call this method normally.

The trigger can then do:

trigger MyObject on MyObject__c (before insert) {
    MyHandler handler = MyHandler.getInstance();

    handler.handleInsert(Trigger.new);
}

We can nicely unit test the actual handler itself, outside the context of the trigger, by doing something like:

@IsTest
class TestMyHandler {
    static Integer handleInsertCalledCount = 0;

    class NoOpHandler extends MyHandler {
        public override void handleInsert(List<MyObject__c> newObjects) {
            handleInsertCalledCount++;
        }
    }

    @IsTest
    static void testInsert() {
        // Turn off all processing in the trigger
        MyHandler.setInstanceType(NoOpHandler.class);

        MyHandler handlerUnderTest = new MyHandler();

        // Do whatever is needed to test the handler handleInsert method. Even if
        // we have to insert MyObject__c instances we know the handler isn't
        // actually called - instead the "no-op" version is used. Since we have a
        // counter that is incremented we can check that the handler is called the
        // expected number of times when the trigger does invoke it, for example
    }
}

This allows for appropriate and true unit testing of code even when actual object inserts/updates/deletes are performed - and all of the latter can have their processing turned off. It is also easy to write variants that throw exceptions or add errors to objects to simulate error cases in order to test behaviours of classes that themselves manage instances of the target object type (MyObject__c here) through DML operations.

This also allows for specific code flows that explicitly disable the trigger processing before DML operations, re-enabling it afterwards like:

MyHandler original = MyHandler.setInstanceType(ProductionNoOpHandler.class);

try {
    insert myObjects;
} finally {
    MyHandler.setInstanceType(original);
}

Where we want to support handler initialization parameters, this can be done using real parameters to the (e.g.) newInstance factory method, where these parameters get turned into an appropriate JSON string that allows us to then use JSON.deserialize to the target type, but that's just a slight variation on this theme.

Fundamentally the fewer static methods you have in your code base the more adaptable, better object-oriented and more properly unit-testable your code truly is. And that's not just in the case of trigger handlers but across the board.

UPDATE: I did forget to mention the Apex Stub API "mocking capability", what little there is of it, can be applied in this pattern too - we have used it a bit though only to support primitive aspects of testing such as invocation counting and method invocation sequence testing.

5

My opinion is that a static method entry point matches the situation well. And if non-static properties are needed as part of the processing inner classes can be used for that with the appropriate lifecycle.

PS

Couple of comments prompted by other answers here:

  • Our job as developers is to deliver business value i.e. the behavior the business needs at a price that makes sense in a way that is open to future change; any abstractions/patterns/frameworks we introduce are only a means to that end
  • Code run from triggers has specific semantics (e.g. what can be modified or have errors added) so abstracting away that context for no strong reason is open to question
  • No future maintainer of the code will complain about you keeping it simple, stupid or often be insightful enough to benefit from an imposed pattern; do the simplest thing to solve the business problem
  • 1
    I agree with much of your sentiment. I do, however, wish to point out that there is a balance between simple code and simple tests. I am aware that Salesforce doesn't promote it due to the way Apex is structured, but true unit testing (i.e. individual class level testing, rather than the "integration testing" approach the typical implementation actually applies) adds significant business benefit - in a more stable solution. Delivering this requires extra code only because Apex does not provide the framework to achieve this OOTB. – Phil W Dec 13 '19 at 7:03
  • @PhilW All great points. – Keith C Dec 13 '19 at 12:25
3

One of the main benefits of the non-static methods is that you can use the Apex Stub API to mock parts of your trigger, which can help with unit tests that depend on data that can't be easily mocked (e.g. a class that depends on the day of the week might benefit from being able to lie about which day it is during a unit test).

Aside from that, several frameworks benefit from having non-static methods (e.g. an abstract class that controls trigger flow, plus non-abstract classes for each trigger). If you have a use for non-static methods, feel free to use them. If you don't plan on using some sort of framework that uses abstraction, using static methods reduces the amount of code you need to write.

Non-static methods will generally require more keystrokes than static methods. Of course, this is minimal in most cases, but if you're an efficient developer, saving keystrokes should be a priority when legibility and performance are not sacrificed as a result.

I think the more important thing is that you should stick with one method instead of mix-and-match. Setting expectations for triggers will make it easier to maintain, and for new developers to jump in on your project. This is a part of the "principle of least astonishment", and following this advice will increase developer efficiency.

2

There is no discernible difference and style preference appears to be the only relevant consideration.

Scenario 1

Trigger:

trigger Demo on Demo__c (before insert)
{
    DemoTriggerHandler.beforeInsert(trigger.new)
}

Handler:

public class DemoTriggerHandler
{
    public static void beforeInsert(List<Demo__c> newRecords)
    {
        while (Limits.getCpuTime() < 1000)
        {
            JSON.deserialize(JSON.serialize('a'.repeat(1000)), String.class);
        }
    }
}

Scenario 2

Trigger:

trigger Demo on Demo__c (before insert)
{
    DemoTriggerHandler handle = new DemoTriggerHandler();
    handle.beforeInsert(trigger.new);
}

Handler:

public class DemoTriggerHandler
{
    public void beforeInsert(List<Demo__c> newRecords)
    {
        while (Limits.getCpuTime() < 1000)
        {
            JSON.deserialize(JSON.serialize('a'.repeat(1000)), String.class);
        }
    }
}

Results

There was a minuscule difference that instance methods are faster, on the order of one to two milliseconds for the whole transaction. I ran each trigger 5 times with a single record, and 5 times with 100 records. The former averaged 1002.6ms and 1000.2ms, respectively, while the latter averaged 1016.2ms and 1015.6ms, respectively. In effect, the difference appears to be nil.

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.