function readOnly(count){ }
Sign Up ›
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
Learn More

Browse by Topic
ShowAll Questionssorted byDate Posted
+ Start a Discussion
SFWesSFWes 

Code Review - 1

I'm fairly new to Apex code and was wondering if someone could help review a trigger for me.

 

Background: We have On-Demand Email-To-Case enabled and will be routing a lot of emails to different addresses in SalesForce

 

What I need is to have the email address that the email was sent to populated on the Case. This will be used for filtering list views in the service console. I couldn't figure out how to get this field on the Case since it resides on the EmailMessage object, so I wrote a Case Email Trigger.

 

I basically just want to know if what I did here is best practice. Any advice would greatly be appreciated.

 

trigger UpdateToField on EmailMessage (after insert) {
    
    List<Case> cases = new List<Case>();
    
    for(EmailMessage em: Trigger.new){
        Case cs = [select SentToAddress__c from Case where Id =: em.ParentId];
        cs.SentToAddress__c = em.ToAddress;
        cases.add(cs);
    }
    update cases;
    
}

 

Best Answer chosen by Admin (Salesforce Developers) 
ryanjuptonryanjupton

SFWes, that is not a recommended best practice. You should never place a SOQL query inside a for loop. Although you can sometimes get away with this if your record count is low, it's begging for trouble later on. What you should look to do instead is something more like this (keep in mind this may not be exactly right in your environment)

 

Map<Id,Case> cases = new Map<Id,Case>([SELECT Id SentToAddress__c FROM Case where ID in(trigger.newmap.keySet())]);
List<Case> casesToUpdate = new List<Case>();
for(EmailCase em : Trigger.new){
	Case c = cases.get(em.ParentId);
	cases.SentToAddress__c = em.ToAddress;
	casesToUpdate.add(c);
}

update cases;

 

All Answers

ryanjuptonryanjupton

SFWes, that is not a recommended best practice. You should never place a SOQL query inside a for loop. Although you can sometimes get away with this if your record count is low, it's begging for trouble later on. What you should look to do instead is something more like this (keep in mind this may not be exactly right in your environment)

 

Map<Id,Case> cases = new Map<Id,Case>([SELECT Id SentToAddress__c FROM Case where ID in(trigger.newmap.keySet())]);
List<Case> casesToUpdate = new List<Case>();
for(EmailCase em : Trigger.new){
	Case c = cases.get(em.ParentId);
	cases.SentToAddress__c = em.ToAddress;
	casesToUpdate.add(c);
}

update cases;

 

This was selected as the best answer
SFWesSFWes

Ryan - Thanks for the information. I know now never to write SOQL inside a for loop.

 

One other question. What happens if that query throws a QueryException? I know I can use a try catch block, but what do i do when the exception is caught? put it to a log?