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
MaheemSamMaheemSam 

Nested For loop Performance Issue

Hi, 

  In below method I am using two for loops inside which is a nested for loop. Code is working as expected but the only issue I see is that this might hit into performance issue when there are bulk operation Please suggest me any alternative way to overcome this issue in the code.  
public static void processOptySharingOppTeam(List<Opportunity> newLst) {
     list<id> OpptIds = new list<id>();
     list<OpportunityTeamMember> Otmlst = new list<OpportunityTeamMember>();
 
     for(Opportunity opp : newLst) {
       OpptIds.add(opp.id); 
      }

  for(Opportunity Op : [select id,name,account.Ultimate_Account__c,partner_account__r.Ultimate_Account__c
                        from opportunity where id in :OpptIds]) {  
            system.debug('Opportunity Name ' + Op.name + '   ' + Op.id);
            
        // Account                             
    for(Opportunity_Sharing__c Ops : [select id, Account_Names__c, Key_Text__c, Partner_Account_Names__c,
                                          User__c,Access_Level__c,Global_Account_Rep__c,Top_Account_ID__r.id, Top_Partner_Account_ID__r.id,
                                          User__r.name
                                          from Opportunity_Sharing__c
                                          where   
                                          Top_Account_ID__r.id <> NULL AND
                                          Top_Account_ID__r.id = :op.account.Ultimate_Account__c]){
                                         
                                
     
               if(Ops.User__c <> null && Ops.Access_Level__c <> null){
                  system.debug('Opportunity Sharing ' + Ops.User__r.name); 
                OpportunityTeamMember otm = new OpportunityTeamMember(OpportunityId = op.id,UserId=Ops.User__c,OpportunityAccessLevel=Ops.Access_Level__c
                                                                              ,TeamMemberRole='Global Account Shared Owner'  ); 
              Otmlst.add(otm);           
             }
         }
      
      // Partner Account                        
      for(Opportunity_Sharing__c Ops : [select id, Account_Names__c, Key_Text__c, Partner_Account_Names__c,
                                            User__c,Access_Level__c,Global_Account_Rep__c,Top_Account_ID__r.id, Top_Partner_Account_ID__r.id,
                                            User__r.name
                                     from Opportunity_Sharing__c
                                     where
                                     Top_Partner_Account_ID__r.id <> NULL AND 
                                     Top_Partner_Account_ID__r.id = :op.partner_account__r.Ultimate_Account__c]){
                                         
                                
     
              if(Ops.User__c <> null && Ops.Access_Level__c <> null){
                 system.debug('Opportunity Sharing ' + Ops.User__r.name); 
               OpportunityTeamMember otm = new OpportunityTeamMember(OpportunityId = op.id,UserId=Ops.User__c,OpportunityAccessLevel=Ops.Access_Level__c
                                                                              ,TeamMemberRole='Global Account Shared Owner'  ); 
              Otmlst.add(otm);           
            }
         }                    
         
      if(!Otmlst.Isempty()){ 
        upsert Otmlst; 
      }        
    }
  }

Thanks
Sudhir
jigarshahjigarshah
Sudhir,

This should help. 
public static void processOptySharingOppTeam(List<Opportunity> newLst) {

    Set<id> OpptIds = new Set<id>();
    list<OpportunityTeamMember> Otmlst = new list<OpportunityTeamMember>();
 
    for(Opportunity opp :newLst){
       OpptIds.add(opp.id); 
    }

	for(Opportunity Op : [Select Id, 
								 Name,
								 Account.Ultimate_Account__c,
								 partner_account__r.Ultimate_Account__c
						  From opportunity 
						  Where Id IN :OpptIds]) {  
						  
        System.debug('Opportunity Name ' + Op.name + '   ' + Op.id);
            
        //Check for matching Accounts or Partner Accounts                             
		for(Opportunity_Sharing__c Ops : [Select Id, 
												 Account_Names__c, 
												 Key_Text__c, 
												 Partner_Account_Names__c,
												 User__c,
												 Access_Level__c,
												 Global_Account_Rep__c,
												 Top_Account_ID__r.id, 
												 Top_Partner_Account_ID__r.id,
												 User__r.name
                                          From Opportunity_Sharing__c
                                          Where   
											(Top_Account_ID__r.id = :op.account.Ultimate_Account__c 
										  OR Top_Partner_Account_ID__r.id = :op.partner_account__r.Ultimate_Account__c)
                                          AND (Ops.User__c <> null 
                                              AND Ops.Access_Level__c <> null)]){
		   
		    System.debug('Opportunity Sharing ' + Ops.User__r.name); 
				
			Otmlst.add(
                new OpportunityTeamMember(
			         OpportunityId = op.id,
				     UserId = Ops.User__c,
				     OpportunityAccessLevel = Ops.Access_Level__c,
				     TeamMemberRole = 'Global Account Shared Owner')
			);
        }
	}//for
			
	//Perform DML outside the loop
	if(!Otmlst.IsEmpty()){ 
		upsert Otmlst; 
	}        
    
}

 
Niraj Kr SinghNiraj Kr Singh
Hi Sudhir
You can try this, I have taken out your SOQL written in inner FOR loop. Because it cause the SOQL limit exception for you.

Note: In your comment might be SOQL limit occur for more than 100 Oppty.

Plz let me know if not working for you. 
public static void processOptySharingOppTeam(List < Opportunity > newLst) {
    list < id > OpptIds = new list < id > ();
	list < OpportunityTeamMember > Otmlst = new list < OpportunityTeamMember > ();
	map< Id, List<Opportunity_Sharing__c> > mapUltAccIdWIthOppShare = new map< Id, List<Opportunity_Sharing__c> >();

    for (Opportunity opp: newLst) {
        OpptIds.add(opp.id);
		
    }
	
	//To get all Opp_Sharing at once.
	for(Opportunity_Sharing__c objOps : [SELECT Id, Account_Names__c, Key_Text__c, Partner_Account_Names__c,
										User__c, Access_Level__c, Global_Account_Rep__c, Top_Account_ID__r.id, Top_Partner_Account_ID__r.Id,
										User__r.name
										FROM Opportunity_Sharing__c
										WHERE
										(Top_Account_ID__r.Id < > null OR Top_Partner_Account_ID__r.Id < > null) AND
										User__c < > null AND
										Access_Level__c < > null]) 
	{
		// Add Account
		if(mapUltAccIdWIthOppShare.containsKey(objOps.Top_Account_ID__r.Id)) {
			mapUltAccIdWIthOppShare.get(objOps.Top_Account_ID__r.Id).add(objOps);
		}
		else {
			mapUltAccIdWIthOppShare.put(objOps.Top_Account_ID__r.Id, new List<Opportunity_Sharing__c>{objOps});
		}
		
		// Add Partner Account
		if(mapUltAccIdWIthOppShare.containsKey(objOps.Top_Partner_Account_ID__r.Id)) {
			mapUltAccIdWIthOppShare.get(objOps.Top_Partner_Account_ID__r.Id).add(objOps);
		}
		else {
			mapUltAccIdWIthOppShare.put(objOps.Top_Partner_Account_ID__r.Id, new List<Opportunity_Sharing__c>{objOps});
		}
		
	}

    for (Opportunity Op: [SELECT Id, Name, account.Ultimate_Account__c, partner_account__r.Ultimate_Account__c
						FROM opportunity 
						WHERE Id IN: OpptIds ]) 
	{
        
		// Account
		if(mapUltAccIdWIthOppShare.containsKey(Op.account.Ultimate_Account__c)) {
			for(Opportunity_Sharing__c Ops : mapUltAccIdWIthOppShare.get(Op.account.Ultimate_Account__c)) {
				Otmlst.add(new OpportunityTeamMember(OpportunityId = op.Id, UserId = Ops.User__c, OpportunityAccessLevel = Ops.Access_Level__c, TeamMemberRole = 'Global Account Shared Owner'));
			}
		}
		
		// Partner Account
		if(mapUltAccIdWIthOppShare.containsKey(Op.partner_account__r.Ultimate_Account__c)) {
			for(Opportunity_Sharing__c Ops : mapUltAccIdWIthOppShare.get(Op.partner_account__r.Ultimate_Account__c)) {
				Otmlst.add(new OpportunityTeamMember(OpportunityId = op.id, UserId = Ops.User__c, OpportunityAccessLevel = Ops.Access_Level__c, TeamMemberRole = 'Global Account Shared Owner'));
			}
		}
	}
	
	if (!Otmlst.Isempty()) {
		upsert Otmlst;
	}
}
Thanks
Niraj
MaheemSamMaheemSam
Thanks for your reply. I used MAP in below code which is working can you see if this is still an issue as nested loop. 

 Also in the code I cannot use OR opearot in SOQL because it will return rows when there is OR condition which is not a valid condition . 
public static void processOptySharingOppTeam(List<Opportunity> newLst) {
      Set<id> actIds = new Set<id>();
      Set<id> partactIds = new Set<id>(); 
      List<id> OpptIds = new List<id>();
      list<OpportunityTeamMember> Otmlst = new List<OpportunityTeamMember>();
      Set<id> Ultactid = new Set<id>();
      Set<id> UltParactid = new Set<id>();
 
     for(Opportunity opp : newLst) {
       OpptIds.add(opp.id); 
      }
      
     list<Opportunity> Oplist = [select id,name,account.Ultimate_Account__c,partner_account__r.Ultimate_Account__c
                                 from opportunity where id in :OpptIds];
                                 
      for(Opportunity Op : Oplist){  
         Ultactid.add(Op.account.Ultimate_Account__c);
         UltParactid.add(Op.partner_account__r.Ultimate_Account__c);                                                     
      }  
      
      map<id,Opportunity_Sharing__c> Actoppshare = new map<id,Opportunity_Sharing__c>([select id, Account_Names__c, Key_Text__c, Partner_Account_Names__c,
                                                                                        User__c,Access_Level__c,Global_Account_Rep__c,Top_Account_ID__r.id, Top_Partner_Account_ID__r.id,
                                                                                        User__r.name
                                                                                       from Opportunity_Sharing__c
                                                                                       where 
                                                                                        Top_Account_ID__r.id <> NULL AND
                                                                                        Top_Account_ID__r.id in :Ultactid ]);
   
      map<id,Opportunity_Sharing__c> Partoppshare = new map<id,Opportunity_Sharing__c>([select id, Account_Names__c, Key_Text__c, Partner_Account_Names__c,
                                                                                       User__c,Access_Level__c,Global_Account_Rep__c,Top_Account_ID__r.id, Top_Partner_Account_ID__r.id,
                                                                                       User__r.name
                                                                                       from Opportunity_Sharing__c
                                                                                       Where Top_Partner_Account_ID__r.id <> NULL AND 
                                                                                             Top_Partner_Account_ID__r.id in :UltParactid ]);
      for(Opportunity Ops : Oplist){
         //Account 
        for(Opportunity_Sharing__c rec : Actoppshare.values()){
            if(Ops.account.Ultimate_Account__c == rec.Top_Account_ID__r.id){
                 system.debug('Opportunity Name: ' + ops.name + '  ' +  ops.id + '  ' +'Opportunity Sharing: ' + + rec.User__r.name);      
                    if(rec.User__c <> null && rec.Access_Level__c <> null){                      
                      OpportunityTeamMember otm = new OpportunityTeamMember(OpportunityId = ops.id,UserId=rec.User__c,OpportunityAccessLevel=rec.Access_Level__c
                                                                              ,TeamMemberRole='Global Account Shared Owner'  ); 
                     Otmlst.add(otm);           
                 }
                  
          }
       }  
        //Partner Account 
        for(Opportunity_Sharing__c rec : Partoppshare.values()){
          if(Ops.partner_account__r.Ultimate_Account__c == rec.Top_Partner_Account_ID__r.id){
                 system.debug('Opportunity Name: ' + ops.name + '  ' +  ops.id + '  ' +'Opportunity Sharing: ' + + rec.User__r.name);    
                    if(rec.User__c <> null && rec.Access_Level__c <> null){                      
                      OpportunityTeamMember otm = new OpportunityTeamMember(OpportunityId = ops.id,UserId=rec.User__c,OpportunityAccessLevel=rec.Access_Level__c
                                                                              ,TeamMemberRole='Global Account Shared Owner'  ); 
                     Otmlst.add(otm);           
                 }            
          }
        }  
     }
     
     if(!Otmlst.Isempty()){ 
        upsert Otmlst; 
      }  
     
  }

Thanks
SAM
Niraj Kr SinghNiraj Kr Singh
Hi Sam,

It will work but It look like you are doing two time SOQL on Opportunity_Sharing__c, Insteed of that you can use one SOQL and combind both where condition together using OR. As I mentioned in my previous comment, it should work for you. You try once the same.
jigarshahjigarshah
Sam,

The below code leverages 2 additional maps oppIdUltAcctIdMap and oppIdUltPartnerAcctIdMap as well leverages the standard OpportuntiyId field on Opportunity_Sharing__c, to eliminate the top level for loop on Oplist.  
public static void processOptySharingOppTeam(List<Opportunity> newLst) {

	Set<Id> oppIdSet = new Set<Id>();
	Map<Id, Set<Id>> oppIdUltAcctIdMap = Map<Id, Set<Id>>();
	Map<Id, Set<Id>> oppIdUltPartnerAcctIdMap = Map<Id, Set<Id>>();
	Set<Id> tempAccIdSet;
	Set<Id> tempPartnerAccIdSet;
	
	Set<id> Ultactid = new Set<id>();
    Set<id> UltParactid = new Set<id>();
	
	List<OpportunityTeamMember> Otmlst = new List<OpportunityTeamMember>();

	//Create a Set of Opp Ids for retrieval
	for(Opportunity oppRec :newLst){
		oppIdSet.add(oppRec);
	}
	
	//Create a map of Opportunity for efficient retrieval
	Map<Id, Opportunity> oppIdRecordMap = new Map<Id, Opportunity>(
		[Select id,name,account.Ultimate_Account__c,partner_account__r.Ultimate_Account__c
		 From opportunity 
		 Where Id IN :oppIdSet]
	);
	
	//Create a Map of Opp Id with appropriate Ultimate Account and PartnerAccount Ids as Values
	for(Id oppId : oppIdRecordMap.keySet()){
	
		//Include Utimate Accounts
		tempAccIdSet = new Set<Id>();
	
		if(oppIdUltAcctIdMap.containsKey(oppId)){
		
			if(oppIdUltAcctIdMap.values().size() > 0){
				tempAccIdSet.addAll(oppIdUltAcctIdMap.values());
			}
		}
		
		//Add the new Account Id
		tempAccIdSet.add(
			((Opportunity)oppIdRecordMap.get(oppId)).account.Ultimate_Account__c;
		);
		oppIdUltAcctIdMap.put(oppId, tempAccIdSet);
		
		
		//Include Partner Accounts
		tempPartnerAccIdSet = new Set<Id>();
		
		if(OppIdUltPartnerAcctIdMap.containsKey(oppId)){
		
			if(OppIdUltPartnerAcctIdMap.values().size() > 0){
				tempPartnerAccIdSet.addAll(OppIdUltPartnerAcctIdMap.values());
			}
		}
		
		//Add the new Account Id
		tempPartnerAccIdSet.add(
			((Opportunity)oppIdRecordMap.get(oppId)).partner_account__r.Ultimate_Account__c;
		);
		oppIdUltAcctIdMap.put(oppId, tempPartnerAccIdSet);
		
		//Create sets for Ultimate Account or Partner Accounts
		Ultactid.add(((Opportunity)oppIdRecordMap.get(oppId)).account.Ultimate_Account__c);
		UltParactid.add((Opportunity)oppIdRecordMap.get(oppId)).partner_account__r.Ultimate_Account__c); 
		
	}//for
	
	//Retrieve Opp Sharing Records with OpportunityId associated with Ultimate Acccount 
	for(Opportunity_Sharing__c oppShareRec :[Select Id, OpportunityId, Account_Names__c, Key_Text__c,  Partner_Account_Names__c,
												User__c,Access_Level__c,Global_Account_Rep__c,Top_Account_ID__r.id, Top_Partner_Account_ID__r.id,
												User__r.name
											From Opportunity_Sharing__c
											Where 
												Top_Account_ID__r.id <> NULL AND
												Top_Account_ID__r.id IN :Ultactid]){

		if(oppIdUltAcctIdMap.get(oppShareRec.OpportunityId).contains(oppShareRec.Top_Account_ID__r.id)
		   && oppShareRec.User__c <> null 
		   && oppShareRec.Access_Level__c <> null){
		
		   Otmlst.add(
				new OpportunityTeamMember(
					OpportunityId = OpportunityId,
					UserId = oppShareRec.User__c,
					OpportunityAccessLevel = oppShareRec.Access_Level__c,
					TeamMemberRole = 'Global Account Shared Owner'
				);
		   ); 
		}
	}//for
	
	//Replicate the same for Partner Accounts
	.
	.
	.
	
	if(Otmlst.size() > 0){ 
        upsert Otmlst; 
    }  
	
}//processOptySharingOppTeam ends
jigarshahjigarshah
Sam,

Has your issue been resolved?  Did the code help?