Apex Code Development You need to sign in to do that
Don't have an account?
Brendan M Making a trigger handler method more efficient?
Hey Community,
I'm a junior developer looking to get a feel for best practice when it comes to trigger helper/handler methods that take take an old and new map then apply some logic in a bulkified and efficient way (0 of N if possible).
Will include my example to see if possible to make more efficient, but if anyone has a better example or template they use it would really help me on my journey?
Task was to improve this method, business process had actually simplified since this was made so it was easier but I'm sure it has lots of room for improvement.
Original method:
public static void assignReopenedCases(Map<ID,Case> oldMap, Map<ID,Case> newMap) {
Case oldCase;
Case newCase;
Set<String> formIDs = new Set<String>{'4','11','14','17','21','24'};
if(Userinfo.getLastName()!=Constants.USER_API_USER) return;
List<Case> cases = [SELECT Id, OwnerId, Cease_Related_to__c, FormName__r.FormID__c, LastQueueID__c FROM Case WHERE Id IN :newMap.keySet()];
Map<String,Id> queuesByName = getQueuesByName();
Map<Id,Id> caseMap = new Map<Id,Id>();
for(Case c : cases) {
newCase = newMap.get(c.Id);
oldCase = oldMap.get(c.Id);
if(oldCase.Status==Constants.CASE_STATUS_CLOSED && newCase.Status==Constants.CASE_STATUS_REOPENED) {
if(newCase.RecordTypeName__c == Constants.CASE_REC_TYPE_OPERATIONS) {
if(formIDs.contains(c.FormName__r.FormID__c) && c.Cease_Related_to__c!=null && c.Cease_Related_to__c.contains('Vehicles')) {
//assign to 'Vehicles'
caseMap.put(c.Id, queuesByName.get(Constants.QUEUE_VEHICLES));
}
else {
//assign to 'General'
caseMap.put(c.Id, queuesByName.get(Constants.QUEUE_GENERAL));
}
}
else if(newCase.RecordTypeName__c == Constants.CASE_REC_TYPE_CMT_RECONCILIATION){
//assign to 'CMT Reconciliation' queue
caseMap.put(c.Id, queuesByName.get(Constants.QUEUE_VEHICLES));
}
else if(newCase.RecordTypeName__c == Constants.CASE_REC_TYPE_CMT_RECOVERY) {
//assign to 'CMT Recovery' queue
caseMap.put(c.Id, queuesByName.get(Constants.QUEUE_CMT_RECOVERIES));
}
else if(c.LastQueueId__c != null && c.OwnerId != queuesByName.get(Constants.QUEUE_VEHICLES)
&& c.OwnerId != queuesByName.get(Constants.QUEUE_GENERAL)
&& newCase.RecordTypeName__c != Constants.CASE_REC_TYPE_SMARTFLEET_CUST_SRV){
//assign to last queue if not assigned to 'Vehicles' or 'General' queue
caseMap.put(c.Id, c.LastQueueId__c);
}
}
}
if(!caseMap.isEmpty()) {
assignCaseToQueue(caseMap);
}
}
My Improved method, looking for further improvements:
public static void assignReopenedCases(Map<ID,Case> oldMap, Map<ID,Case> newMap) {
Case oldCase;
if(Userinfo.getLastName() != Constants.USER_API_USER) return;
Map<Id, Id> caseMap = new Map<Id, Id>();
for (Case newCase : newMap.values()) {
oldCase = oldMap.get(newCase.Id);
if (oldCase.Status == Constants.CASE_STATUS_CLOSED && newCase.Status == Constants.CASE_STATUS_REOPENED) {
if (newCase.LastQueueId__c != null) {
caseMap.put(newCase.Id, newCase.LastQueueId__c);
}
}
}
if (!caseMap.isEmpty()) {
assignCaseToQueue(caseMap);
}
}
Please let me know any changes that are possible! Would love to make it more efficient or remove any further unecessary code.
Thanks community!
To further enhance the efficiency and readability of the assignReopenedCases method:
Eliminate the variable oldCase: Since you're only using it to check the status, you can directly check the status on oldMap in the if condition.
Avoid querying unnecessary fields: You don't need to query fields like OwnerId, Cease_Related_to_c, FormNamer.FormIDc, and RecordTypeName_c since you're not using them in the updated logic.
Remove the formIDs set: Instead of using a set to check if c.FormName_r.FormID_c is in the set, you can directly include that condition in the if statement.
Consider using a switch statement: Instead of a series of if-else if statements based on RecordTypeName__c, you can use a switch statement for better readability.
Here's an updated version incorporating these suggestions:
public static void assignReopenedCases(Map<ID, Case> oldMap, Map<ID, Case> newMap) { if (UserInfo.getLastName() != Constants.USER_API_USER) return; Map<Id, Id> caseMap = new Map<Id, Id>(); for (Case newCase : newMap.values()) { Case oldCase = oldMap.get(newCase.Id); if (oldCase.Status == Constants.CASE_STATUS_CLOSED && newCase.Status == Constants.CASE_STATUS_REOPENED) { if (newCase.LastQueueId__c != null) { switch on newCase.RecordTypeName__c { when Constants.CASE_REC_TYPE_CMT_RECONCILIATION { caseMap.put(newCase.Id, queuesByName.get(Constants.QUEUE_VEHICLES)); } when Constants.CASE_REC_TYPE_CMT_RECOVERY { caseMap.put(newCase.Id, queuesByName.get(Constants.QUEUE_CMT_RECOVERIES)); } when Constants.CASE_REC_TYPE_SMARTFLEET_CUST_SRV { // Do nothing for this record type } when else { if (newCase.FormName_r.FormID_c == '4' || newCase.FormName_r.FormID_c == '11' || newCase.FormName_r.FormID_c == '14' || newCase.FormName_r.FormID_c == '17' || newCase.FormName_r.FormID_c == '21' || newCase.FormName_r.FormID_c == '24') { caseMap.put(newCase.Id, queuesByName.get(Constants.QUEUE_VEHICLES)); } else { caseMap.put(newCase.Id, queuesByName.get(Constants.QUEUE_GENERAL)); } } } } } } if (!caseMap.isEmpty()) { assignCaseToQueue(caseMap); } }If this helps , please mark this as Best Answer.
Thank you.