How to optimize my trigger

Question

I have created a trigger to update a custom date/time field (called First_Aircall_Logged__c). Basically when an (Aircall) task is created on a Case; The trigger updates the Case field First_Aircall_Logged__c with the Task field CreatedDate.

The trigger is working fine but I don't think I wrote it the best way possible. What I really wanted to do was to get tasks where WhatId = CaseId. I am not doing exactly that even though my trigger is working.

How can I optimize my trigger? What would be the best way to write a trigger to update a field of a related object. Thanks

trigger AircallTaskTrigger on Task (after insert, after update) {

//First part of the code: How to query to match WhatId to CaseId
Set<Id> caseIds = new Set<Id>();

Map<Id, Task> aircallTask = new Map<Id, Task>([SELECT Id, WhatId FROM Task WHERE CallDisposition LIKE '%aircall%' AND Id IN: Trigger.New]);
                                                   
for(Task t: aircallTask.values()) { 
    System.debug('##### Aircall Task ' + aircallTask);
    if(t.WhatId != null ) caseIds.add(t.WhatId);
    
}

Map<Id, Case> caseWithAircallTask = new Map<Id, Case> ([SELECT Id, First_Aircall_Logged__c, (SELECT Id, CreatedDate FROM Tasks) FROM Case WHERE Id IN: caseIds]); 
  
if(caseWithAircallTask.size() > 0) { 
    for(Case c :caseWithAircallTask.values()) {
        System.debug('Cases with Aircall Task ' + caseWithAircallTask);
       
         c.First_Aircall_Logged__c = c.tasks[0].CreatedDate;
       
     }
}

update caseWithAircallTask.values();
}

Answer 1

There are a few things that you can consider to optimize this code:

1. No need of querying the Task object in the Task trigger as data will be available in Trigger.New and Trigger.NewMap context variables in both after update and after insert.
2. No need to query Case object as you can create case instances, assign their Id as whatId of task records available in Trigger.New or Trigger.NewMap context variables and can still set the value of First_Aircall_Logged__c on these instances.

After these changes, it will look like below code: **By looking at your code I am assuming that if there are multiple tasks created on a single case then first_Aircall_Logged__c should be populated with latest task's created date.

trigger AircallTaskTrigger on Task (after insert, after update) {

    if(Trigger.isAfter && (Trigger.isInsert || Trigger.isUpdate)){

        Map<Id, Case> caseWithAircallTaskMap = new Map<Id, Case>();                           
        for(Task taskIns : Trigger.New) { 
           
            if(taskIns.whatId != null && String.isNotBlank(taskIns.callDisposition) &&  taskIns.callDisposition.containsIgnoreCase('aircall') {
            
                Case caseIns;
                if(caseWithAircallTaskMap.containsKey(taskIns.whatId)){
                
                    caseIns = caseWithAircallTaskMap.get(taskIns.whatId);
                    if(taskIns.createdDate  > caseIns.first_Aircall_Logged__c){
                        caseIns.first_Aircall_Logged__c = taskIns.createdDate;
                        caseWithAircallTaskMap.put(caseIns.Id, caseIns);
                    }
                }
                else{
                
                    caseIns = new Case(id= taskIns.whatId, first_Aircall_Logged__c = taskIns.createdDate);
                    caseWithAircallTaskList.put(caseIns.Id, caseIns);
                }
                
            }
        }

        update caseWithAircallTaskMap.values();
    }
}

A few good practices you can follow:

1. Use brackets whenever you use if conditions, even when there is only one statement to execute. It makes code more readable and less error prone.
2. Always try to think if you can save some SOQL queries.
3. Always give some meaningful name to variables.
4. Use SOQL queries directly in for loops if possible instead of first assigning them to variable and using that variable in loops, it saves you heap size. But in case you need that data later in the code you will have no other choice other than assigning it to a variable.
5. Try to use camelCase for variable and method names.
6. Please check the context before writing the code in triggers.
7. You can use some exception handling too.

I hope I was able to answer your question.

Answer 2

Your code is actually pretty good, there's just a few optimizations you can make. See the final code at the bottom.

First, we remove debug statements. They're fine for development, but don't leave those in production code.

Second, I changed the logic for the first query into a simple Boolean check. While this does slightly increase CPU time, it saves on queries, which are a precious resource.

Next, we can optimize the queries into a single item, and we add an additional filter so we don't update cases that already have this field set (optional, but probably recommended).

Also, note how I don't worry about the whatId type; null or different record Ids won't return an error or weird records. This saves an If statement.

Finally, no need to check if a list is empty before you loop over it. Iterating over an empty loop is perfectly acceptable. This is a micro-optimization, but it does save typing over the course of many triggers and classes you'll write.

---

trigger AircallTaskTrigger on Task(after insert, after update) {
  Set<Id> whatIds = new Set<Id>();
  for (Task record : Trigger.new) {
    if (
      record.Call_Disposition__c != null &&
      record.Call_Disposition__c.containsIgnoreCase('aircall')
    ) {
      whatIds.add(record.WhatId);
    }
  }
  Case[] caseRecords = [
    SELECT
      (
        SELECT ActivityDate
        FROM Tasks
        WHERE CallDisposition LIKE '%aircall%'
        ORDER BY ActivityDate
        LIMIT 1
      )
    FROM CASE
    WHERE Id IN :whatIds AND First_Aircall_Logged__c = null
  ];
  for (Case record : caseRecords) {
    record.First_Aircall_Logged__c = record.Tasks[0].ActivityDate;
  }
  update caseRecords;
}