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
golugolu 

optimised attachment trigger code

Hi ,

I have written the below trigger code. It is working fine but is not at all optimised. It is having three for loops. Can anyone please guide me with a code snippet. 
trigger AttachmentTrigger on Attachment (after insert) {
    List<Account> accountList = new List<Account>();
    List<Lead> leadlist = new List<Lead>();
    List<String> emails = new List<String>();
    Set<Id> accIds = new Set<Id>();
    List<Attachment> atts = new List<Attachment>();
    for(Attachment att : trigger.New){
        if(att.ParentId.getSobjectType() == Lead.SobjectType){
            accIds.add(att.ParentId);
        }
        
        leadList = [Select id , email from lead where id in : accIds];
        for(Lead l : leadlist){
            emails.add(l.email);
        }
        accountList = [select id, Email__c from Account where email__c in : emails];
        if(accountList!=null && accountList.size()>0){
            for(Account acc : accountList){
                for(Lead ld : leadlist){
                    if(ld.email == acc.email__c){
                        Attachment newFile = new attachment();
                        newfile.Body = att.Body;
                        newfile.Name = att.Name;
                        newfile.Description = att.Description;
                        newfile.ParentId = acc.id;
                        atts.add(newFile);  
                    }
                }
            }
        }
    }
    if(!atts.isEmpty()){
        insert atts;
    }
}

 
Kunal KaushikKunal Kaushik
No need to Write whole logic in first FOR loop.
Check this.

trigger AttachmentTrigger on Attachment (after insert) {
    List<Account> accountList = new List<Account>();
    List<Lead> leadlist = new List<Lead>();
    List<String> emails = new List<String>();
    Set<Id> accIds = new Set<Id>();
    List<Attachment> atts = new List<Attachment>();
    for(Attachment att : trigger.New){
        if(att.ParentId.getSobjectType() == Lead.SobjectType){
            accIds.add(att.ParentId);
        }
    }
        
    leadList = [Select id , email from lead where id in : accIds];
    for(Lead l : leadlist){
        emails.add(l.email);
    }
    accountList = [select id, Email__c from Account where email__c in : emails];
    if(accountList!=null && accountList.size()>0){
        for(Account acc : accountList){
            for(Lead ld : leadlist){
                if(ld.email == acc.email__c){
                    Attachment newFile = new attachment();
                    newfile.Body = att.Body;
                    newfile.Name = att.Name;
                    newfile.Description = att.Description;
                    newfile.ParentId = acc.id;
                    atts.add(newFile);  
                }
            }
        }
    }
    
    if(!atts.isEmpty()){
        insert atts;
    }
}
Ramesh DRamesh D
Hi Golu,
  1. Dont use SOQL quries insode for loop(Read: https://developer.salesforce.com/page/Best_Practice%3A_Avoid_SOQL_Queries_Inside_FOR_Loops)
  2. Try to use Map instead of looping through list of leads to compare with account (Read about Map: https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_methods_system_map.htm) 
trigger AttachmentTrigger on Attachment (after insert) {
    List<Account> accountList = new List<Account>();
    List<Lead> leadlist = new List<Lead>();
    List<String> emails = new List<String>();
    Set<Id> accIds = new Set<Id>();
    List<Attachment> atts = new List<Attachment>();
    Map<id,Attachment> mapOfAtt=new  Map<id,Attachment>();
    Map<string,id> mapOfLeads=new  Map<string,id>();
    for(Attachment att : trigger.New){
        if(att.ParentId.getSobjectType() == Lead.SobjectType){
            accIds.add(att.ParentId);
        }        
        mapOfAtt.put(att.ParentId, att);
        
    }
    leadList = [Select id , email from lead where id in : accIds];
    for(Lead l : leadlist){
        emails.add(l.email);
        mapOfLeads.put(l.email, l.id);
    }
    accountList = [select id, Email__c from Account where email__c in : emails];
    if(accountList!=null && accountList.size()>0){
        for(Account acc : accountList){
           // for(Lead ld : leadlist){
                if(mapOfLeads.containsKey(acc.email__c)){
                    attachment att=mapOfAtt.get(mapOfLeads.get(acc.email__c));
                    Attachment newFile = new attachment();
                    newfile.Body = att.Body;
                    newfile.Name = att.Name;
                    newfile.Description = att.Description;
                    newfile.ParentId = acc.id;
                    atts.add(newFile);  
                }
        }
    }
    
    if(!atts.isEmpty()){
        insert atts;
    }
}

I hope you find the above solution helpful. If it does mark as best answer to help others too.
Thanks,
Ramesh D
 
Ajay K DubediAjay K Dubedi
Hi Golu,
Query in for loop is not best practice and written a helper class of trigger is a best practice part. So try below code i already try this and its works for me:
  
//Trigger

trigger AttachmentTrigger on Attachment (after insert) {
    if(Trigger.isInsert && Trigger.isAfter){
        AttachmentTriggerClass.attachmentTriggerClass_method(trigger.New);
    }
}

//Trigger helper class
public class AttachmentTriggerClass{    
    public static void attachmentTriggerClass_method(List<Attachment> attachmentList){
        List<Account> accountList = new List<Account>();
        List<Lead> leadlist = new List<Lead>();
        List<String> emails = new List<String>();
        //Map<Id, Attachment> leadIdVsAttachment = new Map<Id, Attachment>();
          Map<Id, String> leadIdVsEmail = new Map<Id, String>();
        Set<Id> leadIdSet = new Set<Id>();
        
        List<Attachment> atts = new List<Attachment>();
        Map<String, Id> mapEmailVSId = new Map<String, Id>();
        for(Attachment att : attachmentList){
            if(att.ParentId.getSobjectType() == Lead.SobjectType){
                leadIdSet.add(att.ParentId);
            }
            
        }   
        if(leadIdSet.size() > 0){
            leadList = [SELECT Id , Email from Lead WHERE Id IN: leadIdSet LIMIT 10000];
            for(Lead l : leadlist){
                emails.add(l.email);
                leadIdVsEmail.put(l.Id, l.Email);
            }
        }
        if(emails.size() > 0){
            accountList = [SELECT Id, Email__c from Account WHERE Email__c IN: emails LIMIT 10000];
            if(accountList!=null && accountList.size()>0){
                for(Account accObj : accountList){
                    if(!mapEmailVSId.containsKey(accObj.Email__c)){
                        mapEmailVSId.put(accObj.Email__c, accObj.Id);
                    }
                }
                for(Attachment att : attachmentList){
                    if(leadIdVsEmail.containsKey(att.ParentId) && mapEmailVSId.containsKey(leadIdVsEmail.get(att.ParentId))){
                        String str = leadIdVsEmail.get(att.ParentId);
                        Attachment newFile = new attachment();
                        newfile.Body = att.Body;
                        newfile.Name = att.Name;
                        newfile.Description = att.Description;
                        newfile.ParentId = mapEmailVSId.get(str);
                        atts.add(newFile);
                    }
                }
            }
        }
        
        if(!atts.isEmpty()){
            insert atts;
        }
        
    }
    
}
I hope you find the above solution helpful. If it does, please mark as Best Answer to help others too.
Thanks,
Ajay Dubedi

 
Deepali KulshresthaDeepali Kulshrestha
Hi  Golu,

As best practice says that we can not use a query in a loop so please take care of that.

trigger AttachmentTrigger  on Attachment(before insert) {
    List<Account> accountList = new List<Account>();
    List<Lead> leadlist = new List<Lead>();
    List<String> emails = new List<String>();
    Set<Id> accIds = new Set<Id>();
    List<Attachment> atts = new List<Attachment>();


    Map<Id ,Attachment>attMap = new Map<Id, Attachment>();
    for(Attachment att : trigger.New) {
        if (att.ParentId.getSobjectType() == Lead.SobjectType) {
            accIds.add(att.ParentId);
        }

            attMap.put(att.Id,att);

    }
    leadList = [Select id , email from lead where id in : accIds];
    for(Lead l : leadlist){
        emails.add(l.email);
    }
    accountList = [select id, Email__c from Account where email__c in : emails];

    if(accountList!=null && accountList.size()>0){
        for(Account acc : accountList){
            for(Lead ld : leadlist){
                if(ld.email == acc.email__c){
                    Attachment newFile = new attachment();
                    newfile.Body =attMap.get(ld.Id).Body;
                    newfile.Name = attMap.get(ld.Id).Name;
                    newfile.Description = attMap.get(ld.Id).Description;
                    newfile.ParentId = acc.id;
                    atts.add(newFile);
                }
            }
        }
    }
    if(!atts.isEmpty()){
        insert atts;
    }
}


I hope you find the above solution helpful. If it does, please mark as Best Answer to help others too.

Thanks and Regards,
Deepali Kulshrestha