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
Shree KShree K 

which one to use when Map and List are giving same expected result ?

Hi All, I picked up trigger 1 from trailhead (believing effeciently written ) where Map is used at point in the trigger but I was able to achieve what trigger 1 is doing by using list in trigger 2 (written by me) now what I wonder is I don't see any necessity of using Map in trigger 1.

I am curious to know if both the triggers are effeciently written or only one of them, if so why ?

Trigger - 1
  
trigger AddRelatedRecord1 on Account(after insert) {

    List<Opportunity> oppsToInsrt = new List<Opportunity>();
    // Get the related opportunities for the accounts in this trigger

    Map<Id,Account> acctsWithOpps = new Map<Id,Account>(
        [SELECT Id,Name,(SELECT Id,StageName,CloseDate FROM Opportunities) FROM Account WHERE Id IN :Trigger.New]);
    
    // Add an opportunity for each account if it doesn't already have one.
    // Iterate through each account.
    for(Account a : Trigger.New) {
        System.debug('acctsWithOpps.get(a.Id).Opportunities.size()=' + acctsWithOpps.get(a.Id).Opportunities.size());
        // Check if the account already has a related opportunity.
        if (acctsWithOpps.get(a.Id).Opportunities.size() == 0) {
        
            // If it doesn't, add a default opportunity
            oppsToInsrt.add(new Opportunity(Name=a.Name + ' Opportunity',
                                       StageName='Prospecting',
                                       CloseDate=System.today().addMonths(1),
                                       AccountId=a.Id));
        }           
    }
    if (oppsToInsrt.size() > 0) {
        insert oppsToInsrt;
    }
}

Trigger - 2
  
trigger AddRelatedRecord2 on Account(after insert) {
   
    List<Opportunity> oppList = new List<Opportunity>();  
    
    List<Account> acctsWithOpps = new List<Account>(
        [SELECT Id,Name,city__c,(SELECT Id,stageName,closeDate,city__c FROM Opportunities) FROM Account WHERE Id IN :Trigger.New]);

    for(Account a : acctsWithOpps) {
           
        if (a.Opportunities.size() == 0) {
          
            oppList.add(new Opportunity(Name=a.Name + ' Opportunity',
                                       StageName='Prospecting',
                                       CloseDate=System.today().addMonths(1),
                                       City__c = a.city__c,
                                       AccountId=a.Id));
        }           
    }
    if (oppList.size() > 0) {
        insert oppList;
    }
}

 
Best Answer chosen by Shree K
Andrew GAndrew G
Are both pieces of code efficiently written?  yes.

Are they as efficient as each other?  no.

I would rate trigger 2 as the more efficient code.  The reason is that the first code has the overhead associated with the Map (stack size) and the loop which is doing the .get() call. So loop then get record, where as the second already has the record as part of the loop.

For myself, the benefit of the Map comes when we consider a Loop within a Loop.  I would avoid that scenario and go to Map as ( in very rough maths) ,  the loop within a loop is loop1.size() x loop2.size() iterations where as a Map would be max (loop1.size(),loop2.size)x2 .  As example - 2 lists of 10 and 100 would take 1000 iterations whereas the Map creation (100) plus map loop(100) would be 200 iterations with a little overhead from the .get()

To measure the efficiency in this case, you could create a test class with a large number of records to process and perhaps some debug statements.  You could then check the debug logs to measure the difference.


Regards
Andrew

All Answers

Andrew GAndrew G
Are both pieces of code efficiently written?  yes.

Are they as efficient as each other?  no.

I would rate trigger 2 as the more efficient code.  The reason is that the first code has the overhead associated with the Map (stack size) and the loop which is doing the .get() call. So loop then get record, where as the second already has the record as part of the loop.

For myself, the benefit of the Map comes when we consider a Loop within a Loop.  I would avoid that scenario and go to Map as ( in very rough maths) ,  the loop within a loop is loop1.size() x loop2.size() iterations where as a Map would be max (loop1.size(),loop2.size)x2 .  As example - 2 lists of 10 and 100 would take 1000 iterations whereas the Map creation (100) plus map loop(100) would be 200 iterations with a little overhead from the .get()

To measure the efficiency in this case, you could create a test class with a large number of records to process and perhaps some debug statements.  You could then check the debug logs to measure the difference.


Regards
Andrew
This was selected as the best answer
Shree KShree K
Hi Andrew, Thanks for the quick turnaroud, Now I have another question :)  I have exetended above triggers for update calls(adding Opps to Accts if they don't have one on update event ) where I am not able to avoid a loop inside a loop to add opps to accts(for your quick reference Line 27,32 in trigger 1 and Line 24,27 in trigger 2 ).

Is there any away to better up the triggers by avoiding lop inside loop?

Trigger - 1 (Line or your quick reference )
   
trigger AddRelatedRecord1 on Account(after insert, after update) {

    List<Opportunity> oppsToInsrt = new List<Opportunity>();
    List<Opportunity> oppsToUpdate = new List<Opportunity>();
    // Get the related opportunities for the accounts in this trigger
    Map<Id,Account> acctsWithOpps = new Map<Id,Account>(
        [SELECT Id,Name,(SELECT Id,StageName,CloseDate FROM Opportunities) FROM Account WHERE Id IN :Trigger.New]);
    
    // Add an opportunity for each account if it doesn't already have one.
    // Iterate through each account.
    for(Account a : Trigger.New) {
        System.debug('acctsWithOpps.get(a.Id).Opportunities.size()=' + acctsWithOpps.get(a.Id).Opportunities.size());
        // Check if the account already has a related opportunity.
        if (acctsWithOpps.get(a.Id).Opportunities.size() == 0) {
        
            // If it doesn't, add a default opportunity
            oppsToInsrt.add(new Opportunity(Name=a.Name + ' Opportunity',
                                       StageName='Prospecting',
                                       CloseDate=System.today().addMonths(1),
                                       AccountId=a.Id));
        }           
    }
    if (oppsToInsrt.size() > 0) {
        insert oppsToInsrt;
    }
    
   for(Account a : Trigger.New) {
        System.debug('acctsWithOpps.get(a.Id).Opportunities.size()=' + acctsWithOpps.get(a.Id).Opportunities.size());
        // Check if the account already has a related opportunity.
        if (acctsWithOpps.get(a.Id).Opportunities.size() > 0 ) {
            // If it doesn't, add a default opportunity
          for (Opportunity opps: acctsWithOpps.get(a.Id).Opportunities)
           { 
            
                                       opps.StageName='Prospecting';
                                       opps.CloseDate=System.today().addMonths(1);
                                       oppsToUpdate.add(opps);
           }                            
        }           
    }
    if (oppsToUpdate.size() > 0) {
        Update oppsToUpdate;
    }
    
    
    
}

Trigger - 2
  
trigger AddRelatedRecord2 on Account(after insert, after update) {
   
    List<Opportunity> oppList = new List<Opportunity>();  
    List<Opportunity> oppList1 = new List<Opportunity>();
    
    List<Account> acctsWithOpps = new List<Account>(
        [SELECT Id,Name,city__c,(SELECT Id,stageName,closeDate,city__c FROM Opportunities) FROM Account WHERE Id IN :Trigger.New]);

    for(Account a : acctsWithOpps) {
           
        if (a.Opportunities.size() == 0) {
          
            oppList.add(new Opportunity(Name=a.Name + ' Opportunity',
                                       StageName='Prospecting',
                                       CloseDate=System.today().addMonths(1),
                                       City__c = a.city__c,
                                       AccountId=a.Id));
        }           
    }
    if (oppList.size() > 0) {
        insert oppList;
    }
    
   for(Account a : acctsWithOpps) {
               
        if (a.Opportunities.size() > 0 ) {
        for(opportunity opp:a.opportunities){
        
        opp.StageName='Prospecting';
        opp.CloseDate=System.today().addMonths(1);
        opp.City__c = a.city__c;
        oppList1.add(opp);
        }
           
       
        }           
    }
    if (oppList1.size() > 0) {
        Update oppList1;
    }   
}

 
Andrew GAndrew G
In this case, it would seem that the loop within the loop in unavoidable.   Where we avoid the loop within a loop is when , as a bad business example, I query all Active Accounts and all Active Contacts, and I then loop the Accounts and within that loop , run a loop of all the contacts
e.g.  
for(Account a : activeAccounts) {
    for (Contact c : activeContacts) {
        //do some weird check / code
   }
}
In your case, the only thought i would have is that if i'm loop the accounts already to create the new Oppty, why do a separate loop to do the Oppty updates?
why not : 
for(Account a : acctsWithOpps) {
        if (a.Opportunities.size() == 0) {
            oppList.add(new Opportunity(Name=a.Name + ' Opportunity',
                                       StageName='Prospecting',
                                       CloseDate=System.today().addMonths(1),
                                       City__c = a.city__c,
                                       AccountId=a.Id));
        } else {
            for(opportunity opp:a.opportunities){ 
                opp.StageName='Prospecting'; 
                opp.CloseDate=System.today().addMonths(1); 
                opp.City__c = a.city__c; 
                oppList.add(opp); 
            }
        }            
    }
    if (oppList.size() > 0) {
        upsert oppList;
    }
Shree KShree K
Thank you Andrew. That Helped!