3

UPDATE: If I have more then one field then what is the best way of doing, currently I'm doing the way it showed in the answer:

c.case_dispatch_date__c = csIdToEmailMessageMap.get(c.id).MessageDate;
c.rec_date__c = c = csIdToEmailMessageMap.get(c.id).MessageDate;

I do not like the fact that every time I have to have this line of code csIdToEmailMessageMap.get(c.id).<field>;

END

 for(EmailMessage eMsg : Trigger.new) 
 {  
   List<Case> caseList = [SELECT id FROM Case WHERE Id = :eMsg.ParentId ];
   if (caseList.size()>0)  {
     for(Case c : caseList){ 
       c.case_dispatch_date__c = eMsg.MessageDate;
       update c;
     }
    }
 }

I am trying to update the case dispatch date based on the email sent and I know that I should not be writing update inside the loop, how can I optimized?

1
  • Considering you already have accepted an answer, if you need additional assistance it may be less confusing to ask a separate question.
    – Adrian Larson
    Mar 19 '16 at 0:39
4
  map<ID,EmailMessage> csIdToEmailMessageMap = new map<ID,EmailMessage>();
  for(EmailMessage eMsg : Trigger.new) 
      csIdToEmailMessageMap.put(eMsg.parentId,eMsg); // get parentIds

  List<Case> caseList = [SELECT id FROM Case WHERE Id IN :csIdToEmailMessageMap.keySet() ];
  for(Case c : caseList) 
      c.case_dispatch_date__c = csIdToEmailMessageMap.get(c.id).MessageDate;
  update caseList; 

Above assumes that if two EmailMessage arrives in same batch with the same Case parent, only one will set the case_dispatch_date__c. Which wins is indeterminate but since if in the same batch, it doesn't matter as the MessageDate will be the same day.

12
  • 1
    It's a stylistic difference of opinion, but I think in this case, it would be good to include scope brackets { } around the logic of those two for loop bodies to clearly demonstrate what is part of the loop and what is not.
    – Mark Pond
    Mar 16 '16 at 1:13
  • you are trying to access case_dispatch_date__c without querying
    – Ratan Paul
    Mar 16 '16 at 1:15
  • @MarkPond, I indented a bit more but my personal style is to not use {..} in order to reduce the vertical length of answers. It mirrors the way I code. I know others may claim heresy on this. It is somewhat also a reaction to overly bloated OPs with blank lines, lines containing only { that make the posts so hard to read
    – cropredy
    Mar 16 '16 at 1:16
  • 1
    @Ratan - not an issue, I'm setting the value, not referencing its value
    – cropredy
    Mar 16 '16 at 1:16
  • 2
    you scared me ...no works fine, I just verified in DC.
    – cropredy
    Mar 16 '16 at 1:38
4

first you can collect all the ParentId in set and query case based on set

And store query result in map for further process

Next for loop you can use update case_dispatch_date__c value and perform DML on map values

set<Id> setMessage = new set<Id> ();
for(EmailMessage eMsg : Trigger.new) 
{
  setMessage.add(eMsg.ParentId);
}
map<Id, Case> mapCase = new map<Id, Case>([SELECT id FROM Case WHERE Id IN:setMessage ]);
for(EmailMessage eMsg : Trigger.new) 
{  
   if (mapCase.containskey(eMsg.ParentId))  {
       mapCase.get(eMsg.ParentId).case_dispatch_date__c = eMsg.MessageDate;
    }
 }
 update mapCase.values();
4
  • Ratan -- you changed your moniker? Cool
    – cropredy
    Mar 16 '16 at 14:45
  • @cropredy Thankas. yes i thought it will look good so changed. :-)
    – Ratan Paul
    Mar 16 '16 at 15:41
  • 1+ it was very hard to pick the best answer :-)
    – Nick Kahn
    Mar 16 '16 at 18:11
  • 1
    @NickKahn No problem. glad you solved your problem :-)
    – Ratan Paul
    Mar 16 '16 at 18:12

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.