6

Context

There are a lot of questions and SF developer blogs related to best practices for trigger.

Since several years, I've been following the single trigger object pattern which has a lot of benefit.

But I've recently started working on a project and the Lead Architect insists upon defining a trigger by object and by event like AccountBeforeInsert, AccountBeforeUpdate, etc.

Here's a summary of his reasons

  • When a trigger is defined by event, Salesforce platform naturally routes logic to the appropriate trigger matching the event
  • Not efficient to have single trigger with all possible events and then use if conditions to route logic depending on event (before insert, before event, etc.)
  • Having a trigger by event makes the code more readable
  • The trigger by event should handle the bypass switches logic (via a hierarchical custom settings by object), calls upon a Object Domain class method that performs loop on trigger records and executes the appropriate logic.
  • limit the number of classes called from trigger context to reduce the amount of code retrieved and recompiled when trigger is executed.
  • limit the number of loops on trigger.new to maximum 2 loops

My analysis

I agree that we need to limit number of loops on trigger.new but the rest of the above statements are doubtful because:

  • this suggests too much logic within the trigger itself to check for bypass and then call upon an apex class. It goes against the best practice of having logic-less trigger
  • even if it mentions to limit loop on trigger.new, this model can involve multiple loop on trigger.new records depending on number of methods called by the trigger
  • does not really see how the recommendation for trigger by event simplifies the logic and makes it more efficient

Questions

  1. Do we really have known performances issue with framework like fflib (Force.com Enterprise Architecture) or simpler pattern involving 1 Trigger by object, 1 Handler class by object handling bypass and routing to Service Manager classes by functionality ?
  2. Is amount of code retrieved or recompilation really affected when trigger logic has to drill down to multiple classes ?

This question is targeted to the experts here of SFSE community on the above questions.

Advice on this topic will be greatly appreciated.

2
  • 1
    sfdcfox gives an excellent answer; I would add, when using fflib (which we do in our org) that you should follow/leverage fflib (go all in) as it pays big dividends in unit testing through its natural fit with ApexMocks. fflib already supports some level of trigger event disablement and because it can be extended through sub classes, is easy to add extra features in a DRY manner that all domain classes can use. The multiple passes through Trigger.new is a red herring argument - optimize only if there's a real issue, otherwise, code clarity should take precedence – cropredy Mar 10 '20 at 16:55
  • @cropredy Thanks, we will keep this in mind when using fflib for another project – Shamina Mar 11 '20 at 15:33
11

When a trigger is defined by event, Salesforce platform naturally routes logic to the appropriate trigger matching the event

This is a decent argument in favor of one-trigger-per-object-per-event (OTPOPE), as one-trigger-per-object (OTPO) needs extra logic in place to handle events.

Not efficient to have single trigger with all possible events and then use if conditions to route logic depending on event (before insert, before event, etc.)

That used to be true to some extent, as you'd have to evaluate a chain of if-else statements to call appropriate logic. The "new" switch statement nullifies this argument, as it's now as simple as:

switch on (Trigger.operationType) {
  when BEFORE_INSERT, BEFORE_UPDATE { ... }
  when AFTER_INSERT, AFTER_UPDATE { ... }
  ... etc ...
}

This is just as efficient as the OTPOPE design, but all in one code unit.

Having a trigger by event makes the code more readable

That's opinion. A well-designed single trigger lets a developer easily see all possible trigger events on an object at a glance. I find it easier to read one file than 7 different files, especially if that file is organized with a "table of contents" the way most proper frameworks are.

The trigger by event should handle the bypass switches logic (via a hierarchical custom settings by object), calls upon a Object Domain class method that performs loop on trigger records and executes the appropriate logic.

This is neither an argument for or against this OTPOPE. My framework has a robust custom settings + JIT enabling/disabling of triggers to allow for precise control over one or all triggers, and without much effort, it could be made per event (we don't have this granularity, because we always want all or nothing). Of course, if you're saying the on/off function should be in the trigger itself, that's an argument against OTPOPE, as you now need to maintain up to seven triggers if you modify this logic. A consolidated framework limits this change to just once.

limit the number of classes called from trigger context to reduce the amount of code retrieved and recompiled when trigger is executed.

But up to 7 triggers have to be compiled (given before insert, after insert, before update, after update, before delete, after delete, after undelete)! In a typical OTPO design, it's one trigger and one class (plus any utility classes). For one event, it's a tie, for two or more events, OTPO wins for "fewest compilation units."

Also, not really an argument either way. Classes and triggers are compiled and cached by two levels of code caches (one for the instance, one for each load-balanced unit within the instance), and these caches are very good at what they do. It's unlikely there'd be more than one compilation per deployment or maintenance period. That's far less than a fractional percentage of one percent of an organization's transactions. As it's been said:

Premature optimization is the root of all evil -- DonaldKnuth

Granted, that's not entirely a truism, but for our purposes in this answer, it's close enough. Even if the compilation took 10 seconds, that'd only happen once in a blue moon in most orgs.

limit the number of loops on trigger.new to maximum 2 loops

Maybe. What if it's two loops that are 500 lines long each because someone wanted to stuff all that logic into a single method? Having the logic broken out may mean that you have more oveall loops, but it also means that each loop has a single, defined purpose, and if one of those loops is no longer needed, you can cut it out without disturbing everything else.

Aside from that, it's really rare to need just two loops anyways. Yes, one should group together similar logic to avoid querying the same parent recods, same children records, updating the same records repeatedly, etc, but that's a problem with all designs, not unique to OTPO or OTPOPE.

this suggests too much logic within the trigger itself to check for bypass and then call upon an apex class. It goes against the best practice of having logic-less trigger

Agreed. An ideal trigger is just three lines of code:

trigger X on Y (before insert, before update, before delete, after insert, after update, after delete, after undelete) {
  YtriggerHandler.execute(Trigger.old, Trigger.new, Trigger.operationType);
}

The framework can handle everything else.

even if it mentions to limit loop on trigger.new, this model can involve multiple loop on trigger.new records depending on number of methods called by the trigger

Agreed. The only way to get a maximum of two loops per event is to stuff all your logic inside those two loops, which is likely disastrous for any decently-sized org. This works to some extent, but at some point, the logic will be so complicated that a refactor may be required just to debug an error. If a for loop has 250 lines of code, followed by a second for loop with 250 lines of code, it just may perform better, but I guarantee it's a readability nightmare.

Also, well-designed OTPO frameworks can reuse logic between events (before insert, before update), while keeping other parts separate. This can eliminate a bunch of redundant code and/or reduce the need for a utility class.

does not really see how the recommendation for trigger by event simplifies the logic and makes it more efficient

Efficient isn't always the fewest lines of code. A nested loop can cost exponential amount of time to execute, but a loop with a map is almost always longer code, yet scales in linear time. It's almost always easier for humans to read one file than a bunch of files, assuming they're well-organized.


Do we really have known performances issue with framework like fflib (Force.com Enterprise Architecture) or simpler pattern involving 1 Trigger by object, 1 Handler class by object handling bypass and routing to Service Manager classes by functionality ?

No, we don't. If we did, nobody would use it. It is true there is some loss of efficiency, but that is easily outweighed by maintainability. Truth is, not using patterns such as fflib tend to make code slower, because fflib and the like are "solved problems" and let you get on with just writing your business logic in an "efficient-enough" manner.

Is amount of code retrieved or recompilation really affected when trigger logic has to drill down to multiple classes ?

Nope. Recompilation typically only happens after a deployment or a maintenance, so it's incredibly rare and usually not worth mentioning. Also, again, if that's an argument to be made, OTPOPE makes things worse, not better, because now you have multiple triggers to compile, too.


Overall, there's not many good points for OTPOPE vs. OTPO. The latter lends itself to better maintenance and more readable code. That said, both designs can be abused, giving the appearance that one is better than the other. Conceptually, there's so minimal a difference it doesn't matter. However, OTPO is recommended by Salesforce engineers, so who are we to argue?

Also, it's so common these days that OTPOPE may confuse new-hires, and overall cause more work to fix problems. Also, it leads to some philosophical issues, like what happens when you need to store data between a before insert and after insert trigger? Does it go to a neutral third class? Or one of the other classes? A service class? Having all trigger events in the same class makes this decision simple and consistent.

I would say, though, that if that's the team's direction, OTPOPE, then that's how it is. I would just recommend avoiding mixing the two, as different design philosophies in the same org can create maintenance nightmares.

2
  • 3
    This is something everyone needs to focus - "That said, both designs can be abused, giving the appearance that one is better than the other.". Unless the design/approach/practice is all accounted for, there is always a loophole. – Jayant Das Mar 10 '20 at 15:02
  • @sfdcfox Excellent answer indeed, this is very insightful, I really appreciate such detailed analysis in your answer :) ...Totally agree that OTPOPE approach confuses junior developers, this is one of the difficulties we are facing. Unfortunately, we have to adapt ourselves to the OTPOPE approach for this project because we do not have our say on this design and our arguments are ignored. – Shamina Mar 11 '20 at 15:30

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.