function readOnly(count){ }
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
+ Start a Discussion
Mhlangano KhumaloMhlangano Khumalo 

Help with for-loop bulkification: I want to remove an SQL query inside a for loop

I want pull only the list of records that match the ones i'm getting through a JSON array list. The json array list string doesn't have the Salesforce record ID.The JSON string looks like below. I use the rcNumber, accountNumber and bankName  as a composite primary key and what's returned on unpaidCode as a condition for updating records.
{
    "bacctList": [{
        "rcNumber": "RC-2848323",
        "accountNumber": "12324434",
        "bankName" :  "NEDBANK",
        "unpaidCode" : ""
    },
    {
        "rcNumber": "RC-002274",
        "accountNumber": "1111111",
        "bankName" :  "FNB",
        "unpaidCode" : "99"
    }]
}
The Logic of the class works perfectly, I just want to bulkify it. Please help!...the fist for loop (line 27) with an inner loop, I think is not best practice.
 
@RestResource(urlMapping='/dosystem/bankaccounts2/*')
global with sharing class BankAccountWebservice2 {
       
    //Decorator class- Retured JSON mapping
    global class BankAccountsReturned{
        public String rcNumber;             
        public String accountNumber;  
        public String bankName;
        public String unpaidCode;
        
        public BankAccountsReturned(String rcNum, String accNum, String bName, String uCode){
            this.rcNumber = rcNum;
            this.accountNumber = accNum;
            this.bankName = bName;
            this.unpaidCode =uCode;
        }          
    } 
    
    @HttpPatch
    global static List<Bank_Account__c > updateDateProcessed(BankAccountsReturned [] bacctList) {
        
        //St
        List<String> rcNumbers = new List<String>{};  
           
        List<Bank_Account__c> BAccList  = new List<Bank_Account__c>{};
        
        for (integer i=0; i< bacctList.size(); i++) {                                                         
             for(Bank_Account__c j : [select id, Account__r.Name, Account__r.RC_Account_No__c,Bank__c,RC_Account_No__c,Unpaid_Code__c  
                                                         FROM Bank_Account__c where 
                                                         RC_Account_No__c =: bacctList[i].rcNumber AND 
                                                         A_c_Number__c =: bacctList[i].accountNumber AND 
                                                         Bank__c =: bacctList[i].bankName ]){

              // If unpaidCode is blank then deactive bank account, DO NOT create case 
              if(bacctList[i].unpaidCode == ''){                      
                  BAccList.add(new Bank_Account__c(Id = j.Id, Debit_Order_A_c__c = false, Unpaid_Code__c = bacctList[i].unpaidCode));
              }
              /* If the error code is populated with 99 the DO NOT deactivate the bank account BUT
               create an active ADHOC record with the current Debit Order amount on the bank account. DO NOT Create Case */
              else if(bacctList[i].unpaidCode == '99'){
                  rcNumbers.add(j.Account__r.RC_Account_No__c);
                  BAccList.add(new Bank_Account__c(Id = j.Id, Debit_Order_A_c__c = true, Unpaid_Code__c = bacctList[i].unpaidCode));                  
              }
              //If unpaid code is any other number beside 99, deactivate and create a case
              else{
                 
                  rcNumbers.add(j.Account__r.RC_Account_No__c); //<--Populates the list with ONLY RC Numbers we need to create cases for
                  BAccList.add(new Bank_Account__c(Id = j.Id, Debit_Order_A_c__c = false, Unpaid_Code__c = bacctList[i].unpaidCode));
              
              }
            }                                  
        }  
        
        update  BAccList;
        
        //Create Adhoc records
        List<Ad_hoc_Debit_Orders__c> adhocRecords = new List<Ad_hoc_Debit_Orders__c>();
        for(Bank_Account__c createActiveDO : [select id, Bank__c,Unpaid_Code__c,
                                            (select Id,Current_DO__c from Bank_Account__c.Debit_Orders__r where Active__c = true Order by CreatedDate Desc LIMIT 1)
                                              from Bank_Account__c where id in: BAccList and Unpaid_Code__c= '99']){
                                              
            Ad_hoc_Debit_Orders__c ad = new Ad_hoc_Debit_Orders__c(); 
            ad.Bank_ID__c = createActiveDO.id;
            ad.Custom__c = 'Adhoc';
            ad.Debit_Order_amount__c = createActiveDO.Debit_Orders__r[0].Current_DO__c;
            ad.Active__c = true;   
            adhocRecords.add(ad);     
        }
        insert adhocRecords;
        
        
        //Get more details about the deactivated bank account inoder to be populated when creating case.
        List<Case> createCase = new List<Case>();
        for(Account oUsr: [SELECT Id, Name, RC_Account_No__c,
                                    (select Id, Name, email, phone from contacts where Primary_contact_Indicator__c = true order by createddate desc Limit 1),
                                    (select Id, Name, Bank__c from Account.Bank_Accounts__r where Debit_Order_A_c__c = true Order by CreatedDate Desc LIMIT 1 )
                                     FROM Account WHERE RC_Account_No__c in: rcNumbers ]){
            Case u = new Case();            
            u.AccountId = oUsr.id;
            u.ContactId = oUsr.Contacts[0].Id;        
            u.Status = 'New';
            u.Origin = 'API Call';
            u.Subject = 'Deactivated Bank Account';
            u.Description = 'Bank Account Deactivated for '+oUsr.Name +' Account with  '+oUsr.RC_Account_No__c+' Number';             
            createCase.add(u);
        }
        insert createCase;
       
       return BAccList;
       }         
    }
I would like to use best practice. Because the json array string doesn't have the salesforce record Id, I want to select only the records is salesforce that = my composite primary key. and store them on a list as it will have the Record ID which will make it easier to do processing & updates.
Best Answer chosen by Mhlangano Khumalo
Keyur  ModiKeyur Modi
Hi Khumalo,

I have made changes in code and I have taken three set and one map to remove for inside for and query inside for loop.

Here in below I have assumed that account number(bacctList[i].accountNumber) is unique ,but if it is not unique and anything else is unique then you need to take that field as key in map.
 
@RestResource(urlMapping='/dosystem/bankaccounts2/*')
global with sharing class BankAccountWebservice2 {
       
    //Decorator class- Retured JSON mapping
    global class BankAccountsReturned{
        public String rcNumber;             
        public String accountNumber;  
        public String bankName;
        public String unpaidCode;
        
        public BankAccountsReturned(String rcNum, String accNum, String bName, String uCode){
            this.rcNumber = rcNum;
            this.accountNumber = accNum;
            this.bankName = bName;
            this.unpaidCode =uCode;
        }          
    } 
    
    @HttpPatch
    global static List<Bank_Account__c > updateDateProcessed(BankAccountsReturned [] bacctList) {
        
        //St
        List<String> rcNumbers = new List<String>{};  
		
		Set<String> setbankName = new set<String>();//select data type of set based on field data type 
		Set<String> setAccNumber = new set<String>();//select data type of set based on field data type
		Set<String> setRcNumber = new set<String>(); //select data type of set based on field data type
		map<String,Bank_Account__c> mapOfBankAcc = new map<String,Bank_Account__c>(); //map to get data from query keep key data type based on A_c_Number__c field data type 
        List<Bank_Account__c> BAccList  = new List<Bank_Account__c>{};
		
		for(integer i=0; i< bacctList.size(); i++){
				setbankName.add(bacctList[i].bankName);
				setAccNumber.add(bacctList[i].accountNumber);
				setRcNumber.add(bacctList[i].rcNumber);
		}
		
		for(Bank_Account__c j : [select id, Account__r.Name, Account__r.RC_Account_No__c,Bank__c,RC_Account_No__c,Unpaid_Code__c  
                                                         FROM Bank_Account__c where 
                                                         RC_Account_No__c IN: setRcNumber AND 
                                                         A_c_Number__c IN: setAccNumber AND 
                                                         Bank__c  IN: setbankName ]){
		
				mapOfBankAcc.put(j.A_c_Number__c,j);
		}
        
        for (integer i=0; i< bacctList.size(); i++) {                                                         
            

              // If unpaidCode is blank then deactive bank account, DO NOT create case 
              if(bacctList[i].unpaidCode == ''){                      
                  BAccList.add(new Bank_Account__c(Id = mapOfBankAcc.get(bacctList[i].accountNumber).Id, Debit_Order_A_c__c = false, Unpaid_Code__c = bacctList[i].unpaidCode));
              }
              /* If the error code is populated with 99 the DO NOT deactivate the bank account BUT
               create an active ADHOC record with the current Debit Order amount on the bank account. DO NOT Create Case */
              else if(bacctList[i].unpaidCode == '99'){
                  rcNumbers.add(mapOfBankAcc.get(bacctList[i].accountNumber).Account__r.RC_Account_No__c);
                  BAccList.add(new Bank_Account__c(Id = mapOfBankAcc.get(bacctList[i].accountNumber).Id, Debit_Order_A_c__c = true, Unpaid_Code__c = bacctList[i].unpaidCode));                  
               }
              //If unpaid code is any other number beside 99, deactivate and create a case
              else{
                 
                  rcNumbers.add(mapOfBankAcc.get(bacctList[i].accountNumber).Account__r.RC_Account_No__c); //<--Populates the list with ONLY RC Numbers we need to create cases for
                  BAccList.add(new Bank_Account__c(Id = mapOfBankAcc.get(bacctList[i].accountNumber).Id, Debit_Order_A_c__c = false, Unpaid_Code__c = bacctList[i].unpaidCode));
              
              }
                                              
        }  
        
        update  BAccList;
        
        //Create Adhoc records
        List<Ad_hoc_Debit_Orders__c> adhocRecords = new List<Ad_hoc_Debit_Orders__c>();
        for(Bank_Account__c createActiveDO : [select id, Bank__c,Unpaid_Code__c,
                                            (select Id,Current_DO__c from Bank_Account__c.Debit_Orders__r where Active__c = true Order by CreatedDate Desc LIMIT 1)
                                              from Bank_Account__c where id in: BAccList and Unpaid_Code__c= '99']){
                                              
            Ad_hoc_Debit_Orders__c ad = new Ad_hoc_Debit_Orders__c(); 
            ad.Bank_ID__c = createActiveDO.id;
            ad.Custom__c = 'Adhoc';
            ad.Debit_Order_amount__c = createActiveDO.Debit_Orders__r[0].Current_DO__c;
            ad.Active__c = true;   
            adhocRecords.add(ad);     
        }
        insert adhocRecords;
        
        
        //Get more details about the deactivated bank account inoder to be populated when creating case.
        List<Case> createCase = new List<Case>();
        for(Account oUsr: [SELECT Id, Name, RC_Account_No__c,
                                    (select Id, Name, email, phone from contacts where Primary_contact_Indicator__c = true order by createddate desc Limit 1),
                                    (select Id, Name, Bank__c from Account.Bank_Accounts__r where Debit_Order_A_c__c = true Order by CreatedDate Desc LIMIT 1 )
                                     FROM Account WHERE RC_Account_No__c in: rcNumbers ]){
            Case u = new Case();            
            u.AccountId = oUsr.id;
            u.ContactId = oUsr.Contacts[0].Id;        
            u.Status = 'New';
            u.Origin = 'API Call';
            u.Subject = 'Deactivated Bank Account';
            u.Description = 'Bank Account Deactivated for '+oUsr.Name +' Account with  '+oUsr.RC_Account_No__c+' Number';             
            createCase.add(u);
        }
        insert createCase;
       
       return BAccList;
       }         
    }

Please try above code and let me know if you have any query related to this code then.

Thanks,
Keyur Modi

All Answers

Keyur  ModiKeyur Modi
Hi Khumalo,

I have made changes in code and I have taken three set and one map to remove for inside for and query inside for loop.

Here in below I have assumed that account number(bacctList[i].accountNumber) is unique ,but if it is not unique and anything else is unique then you need to take that field as key in map.
 
@RestResource(urlMapping='/dosystem/bankaccounts2/*')
global with sharing class BankAccountWebservice2 {
       
    //Decorator class- Retured JSON mapping
    global class BankAccountsReturned{
        public String rcNumber;             
        public String accountNumber;  
        public String bankName;
        public String unpaidCode;
        
        public BankAccountsReturned(String rcNum, String accNum, String bName, String uCode){
            this.rcNumber = rcNum;
            this.accountNumber = accNum;
            this.bankName = bName;
            this.unpaidCode =uCode;
        }          
    } 
    
    @HttpPatch
    global static List<Bank_Account__c > updateDateProcessed(BankAccountsReturned [] bacctList) {
        
        //St
        List<String> rcNumbers = new List<String>{};  
		
		Set<String> setbankName = new set<String>();//select data type of set based on field data type 
		Set<String> setAccNumber = new set<String>();//select data type of set based on field data type
		Set<String> setRcNumber = new set<String>(); //select data type of set based on field data type
		map<String,Bank_Account__c> mapOfBankAcc = new map<String,Bank_Account__c>(); //map to get data from query keep key data type based on A_c_Number__c field data type 
        List<Bank_Account__c> BAccList  = new List<Bank_Account__c>{};
		
		for(integer i=0; i< bacctList.size(); i++){
				setbankName.add(bacctList[i].bankName);
				setAccNumber.add(bacctList[i].accountNumber);
				setRcNumber.add(bacctList[i].rcNumber);
		}
		
		for(Bank_Account__c j : [select id, Account__r.Name, Account__r.RC_Account_No__c,Bank__c,RC_Account_No__c,Unpaid_Code__c  
                                                         FROM Bank_Account__c where 
                                                         RC_Account_No__c IN: setRcNumber AND 
                                                         A_c_Number__c IN: setAccNumber AND 
                                                         Bank__c  IN: setbankName ]){
		
				mapOfBankAcc.put(j.A_c_Number__c,j);
		}
        
        for (integer i=0; i< bacctList.size(); i++) {                                                         
            

              // If unpaidCode is blank then deactive bank account, DO NOT create case 
              if(bacctList[i].unpaidCode == ''){                      
                  BAccList.add(new Bank_Account__c(Id = mapOfBankAcc.get(bacctList[i].accountNumber).Id, Debit_Order_A_c__c = false, Unpaid_Code__c = bacctList[i].unpaidCode));
              }
              /* If the error code is populated with 99 the DO NOT deactivate the bank account BUT
               create an active ADHOC record with the current Debit Order amount on the bank account. DO NOT Create Case */
              else if(bacctList[i].unpaidCode == '99'){
                  rcNumbers.add(mapOfBankAcc.get(bacctList[i].accountNumber).Account__r.RC_Account_No__c);
                  BAccList.add(new Bank_Account__c(Id = mapOfBankAcc.get(bacctList[i].accountNumber).Id, Debit_Order_A_c__c = true, Unpaid_Code__c = bacctList[i].unpaidCode));                  
               }
              //If unpaid code is any other number beside 99, deactivate and create a case
              else{
                 
                  rcNumbers.add(mapOfBankAcc.get(bacctList[i].accountNumber).Account__r.RC_Account_No__c); //<--Populates the list with ONLY RC Numbers we need to create cases for
                  BAccList.add(new Bank_Account__c(Id = mapOfBankAcc.get(bacctList[i].accountNumber).Id, Debit_Order_A_c__c = false, Unpaid_Code__c = bacctList[i].unpaidCode));
              
              }
                                              
        }  
        
        update  BAccList;
        
        //Create Adhoc records
        List<Ad_hoc_Debit_Orders__c> adhocRecords = new List<Ad_hoc_Debit_Orders__c>();
        for(Bank_Account__c createActiveDO : [select id, Bank__c,Unpaid_Code__c,
                                            (select Id,Current_DO__c from Bank_Account__c.Debit_Orders__r where Active__c = true Order by CreatedDate Desc LIMIT 1)
                                              from Bank_Account__c where id in: BAccList and Unpaid_Code__c= '99']){
                                              
            Ad_hoc_Debit_Orders__c ad = new Ad_hoc_Debit_Orders__c(); 
            ad.Bank_ID__c = createActiveDO.id;
            ad.Custom__c = 'Adhoc';
            ad.Debit_Order_amount__c = createActiveDO.Debit_Orders__r[0].Current_DO__c;
            ad.Active__c = true;   
            adhocRecords.add(ad);     
        }
        insert adhocRecords;
        
        
        //Get more details about the deactivated bank account inoder to be populated when creating case.
        List<Case> createCase = new List<Case>();
        for(Account oUsr: [SELECT Id, Name, RC_Account_No__c,
                                    (select Id, Name, email, phone from contacts where Primary_contact_Indicator__c = true order by createddate desc Limit 1),
                                    (select Id, Name, Bank__c from Account.Bank_Accounts__r where Debit_Order_A_c__c = true Order by CreatedDate Desc LIMIT 1 )
                                     FROM Account WHERE RC_Account_No__c in: rcNumbers ]){
            Case u = new Case();            
            u.AccountId = oUsr.id;
            u.ContactId = oUsr.Contacts[0].Id;        
            u.Status = 'New';
            u.Origin = 'API Call';
            u.Subject = 'Deactivated Bank Account';
            u.Description = 'Bank Account Deactivated for '+oUsr.Name +' Account with  '+oUsr.RC_Account_No__c+' Number';             
            createCase.add(u);
        }
        insert createCase;
       
       return BAccList;
       }         
    }

Please try above code and let me know if you have any query related to this code then.

Thanks,
Keyur Modi
This was selected as the best answer
UC InnovationUC Innovation
Best practices basically says to query for everything you need and then use a loop to process the records.  In your case, I think it's sufficient to switch the first two loops around?
 
So, instead of looping through the JSON data and then querying for right Bank_Account__c records, just query for all the Bank_Account__c records and then loop through the JSON data to see if there's a match.
Mhlangano KhumaloMhlangano Khumalo
@Keyur,

Thanks, solution works.