Apex Code Development You need to sign in to do that
Don't have an account?
less logic trigger
Hey guys,
i have read several articels about best practice for triggers. Most of the 'rules' are easaly to understand and logical.
One of the rules is 'less logic trigger'. I understand that it doesn't make sense to write very complex triggers. But in my opinion, when i have one trigger per object, at least i have to tell trigger when he needs to fire the specific classes.
To be more specific:
I have three classes, each of them should be fired after a specific update:
- 1 classe creates a copy of the opportunity (fire after closed won)
- 1 class creates different tasks (fire after the owner has changed)
- 1 class for an apex callout (fire after field is updated for the first time)
- 1 class for handle recursion
All of these classes are related to the opportunity object. So i have to create one trigger with the context variable (after update). Sometimes i can handle the logic within the class but often i need the trigger.oldmap variable to make sure, that the trigger act like it should.
so my trigger would look sth like
i have read several articels about best practice for triggers. Most of the 'rules' are easaly to understand and logical.
One of the rules is 'less logic trigger'. I understand that it doesn't make sense to write very complex triggers. But in my opinion, when i have one trigger per object, at least i have to tell trigger when he needs to fire the specific classes.
To be more specific:
I have three classes, each of them should be fired after a specific update:
- 1 classe creates a copy of the opportunity (fire after closed won)
- 1 class creates different tasks (fire after the owner has changed)
- 1 class for an apex callout (fire after field is updated for the first time)
- 1 class for handle recursion
All of these classes are related to the opportunity object. So i have to create one trigger with the context variable (after update). Sometimes i can handle the logic within the class but often i need the trigger.oldmap variable to make sure, that the trigger act like it should.
so my trigger would look sth like
trigger opptrigger on Opportunity (after update) {
for (Opportunity opp : trigger.new){
if(!HelperClassTrigger.SetOfIDs.contains(opp.Id)){
Opporuntiy oldOpp = trigger.oldMap.get(opp.Id)
if( !oldOpp.isWon && opp.isWon){
// class for copy creation
}
if( !oldOpp.OwnerId != opp.OwnerId){
// class for owner change
}
if(String.IsEmpty(oldOpp.Field__c)
&& !String.IsEmpty(old.Field__c)){
// class for callout
}
}
}
There is another way of writing trigger with Domain Layer and patterns such as Data Mapper, Service Layer and Unit of Work.
https://github.com/financialforcedev/fflib-apex-common
/** * Base class aiding in the implementation of a Domain Model around SObject collections * * Domain (software engineering). “a set of common requirements, terminology, and functionality * for any software program constructed to solve a problem in that field”, * http://en.wikipedia.org/wiki/Domain_(software_engineering) * * Domain Model, “An object model of the domain that incorporates both behavior and data.”, * “At its worst business logic can be very complex. Rules and logic describe many different " * "cases and slants of behavior, and it's this complexity that objects were designed to work with...” * Martin Fowler, EAA Patterns * http://martinfowler.com/eaaCatalog/domainModel.html * **/ public virtual with sharing class fflib_SObjectDomain implements fflib_ISObjectDomain { /** * For Domain classes implementing the ITriggerStateful interface returns the instance * of the domain class being shared between trigger invocations, returns null if * the Domain class trigger has not yet fired or the given domain class does not implement * the ITriggerStateful interface. Note this method is sensitive to recursion, meaning * it will return the applicable domain instance for the level of recursion **/ public static fflib_SObjectDomain getTriggerInstance(Type domainClass) { List<fflib_SObjectDomain> domains = TriggerStateByClass.get(domainClass); if(domains==null || domains.size()==0) return null; return domains[domains.size()-1]; } /** * Detects whether any values in context records have changed for given fields as strings * Returns list of SObject records that have changes in the specified fields **/ public List<SObject> getChangedRecords(Set<String> fieldNames) { List<SObject> changedRecords = new List<SObject>(); for(SObject newRecord : Records) { Id recordId = (Id)newRecord.get('Id'); if(Trigger.oldMap == null || !Trigger.oldMap.containsKey(recordId)) continue; SObject oldRecord = Trigger.oldMap.get(recordId); for(String fieldName : fieldNames) { if(oldRecord.get(fieldName) != newRecord.get(fieldName)) changedRecords.add(newRecord); } } return changedRecords; }https://github.com/financialforcedev/fflib-apex-common/blob/master/fflib/src/classes/fflib_SObjectDomain.cls
https://trailhead.salesforce.com/en/content/learn/modules/apex_patterns_dsl/apex_patterns_dsl_learn_dl_principles
Not sure that could solve your problem but this framework deals with a domain class being shared between trigger invocations and the recursions as well as the changed records more "easily".
thanks for your help ! This solution looks more complex. Is it really best practice to use it ? I definitely will do the trailhead modules but of course i want to deploy my code as soon as possible (considering best practice). If you want and if you have time for that i could share my whole code with you.
You have a single trigger and different states for the opportunity, how to write the code when you prevent the recursion? (by definition the next action is lost if the trigger blocks the recursion too early). I don't have the written solution either.
Your question was about the best practices for a trigger and these enterprise patterns are one of the best options but that must be installed at the beginning of a project ideally (separation of concerns).
The final result for a trigger is very surprising with these patterns because all the triggers contain only ... one line (all the treatment is relocated in other classes).
i dont know if you remember but its not the first time you have helped me :D In my opion its simple to make the trigger work like it should, but i feel a bit uncomfortable about the 'less trigger logic' stuff.
So i have a simple to helper class to prevent recursion
public Class HelperClassTrigger{ public static Set<Id> SetOfIDs = new Set<Id>(); }My trigger:
trigger OpportunityClosedWon on Opportunity (after update) { // for loop is not necessary Opportunity opp = Trigger.new[0]; Opportunity oldOpp = Trigger.oldMap.get(opp.Id); // best practice to filter recordtype Id recTypeId = Schema.getGlobalDescribe().get('Opportunity').getDescribe().getRecordTypeInfosByName().get('Kolbermoor Opportunity').getRecordTypeId(); Boolean push2Slack = opp.Slack__c && !oldOpp.Slack__c; Boolean updateLost = string.isBlank(oldOpp.Lost_Customer_Reason__c); Boolean won = opp.isWon && !oldOpp.IsWon & oldOpp.Time_80_Closed_Won__c == Null; if(Trigger.isUpdate && opp.RecordTypeId.equals(recTypeId)){ //check to prevent recursion if (!HelperClassTrigger.SetOfIDs.contains(opp.Id)) { if (push2Slack){// methode to create slack message PushOpportunityToSlackChannel.postToSlack(opp.id); HelperClassTrigger.SetOfIDs.add(opp.Id); } if (updateLost) {// methode to update renewal task UpdateRenewalTask.updateTask(opp.id); HelperClassTrigger.SetOfIDs.add(opp.Id); } if(won){ CreateRenewalOpportunity.cloneOpportunity(opp.id);//methode to create renewal opportunity CreateTask4Template.createTaskList(opp.id);//methode to create list of tasks HelperClassTrigger.SetOfIDs.add(opp.Id); } } } }So for each update transaction the trigger checks if it fits some values, if it does, the trigger calls the methode and add the id to the set.
All the triggers that use a domain class instance are just like below:
trigger OpportunitiesTrigger on Opportunity ( after delete, after insert, after update, after undelete, before delete, before insert, before update) { // Creates Domain class instance and calls appropriate methods fflib_SObjectDomain.triggerHandler(Opportunities.class); }fflib_SObjectDomain.triggerHandler(Opportunities.class); // one line (always the same) only the parameter (domain class) changes.
The domain class Opportunities.class is instanciated by using the System.Type class and its inner class Constructor (fixed name).
https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_methods_system_type.htm
It is also a Logic-less Trigger (no code inside the trigger itself). It is a standardization of the code but your solution is sufficient.
https://trailhead.salesforce.com/en/content/learn/modules/apex_patterns_dsl/apex_patterns_dsl_learn_dl_principles
Here is an example of how I simplify my triggers.
https://developer.salesforce.com/forums/ForumsMain?id=9062I000000IK8dQAG
Whilst the post was actually about the test code, the trigger example should give you an alternate idea.
My triggers generally have the same simple code to call the handler - eg objHandler.handleBeforeInsert(Trigger.New);
In this way, my triggers are always the same (or similar) structure, with (generally) only the declaration of the objHandler changing between triggers.
Each handler starts with the event handler type method e.g. public void handleBeforeInsert(List<Case> newCases)
which then calls the required actual methods to handle each type of activity.
In this way, I can keep one trigger per object, and one trigger handler per object. With all the code in one handler, I can better handle the execution order, which reduces the chance of any recursion issues. If you already have code in other classes, you could simply call them as required in each event method.
Regards
Andrew