Rewrite apex class to be out of the loop

Question

I have an apex class that I have been having trouble with when I try to upload in bulk accounts. It is tied to triggers that do certain things depending on whether the Customer_Success_Manager__c field is filled in. I've edited this class several times to get as much out of the loop as I can get but I am still getting this error. I am new to development and I need help to get this to a point where it the code can handle bulk requests. I also would like to know how do you know when too much is in the loop. If you answer with code it would be great if you could leave an explanation of what the code is saying so that I can apply that logic in the future. The apex class is below.

public class UpdateCSM{        
    public static void updateTeamMember(List<Account> accounts){            
        List<AccountShare> ashareLIST = new List<AccountShare>();            
        List<AccountTeamMember> ATM = new List<AccountTeamMember>();            
        Set<String> rmMember = new Set<String>();            
        Map<ID, ID> acctToBRepMap = new Map<ID, ID>();            

        for(Account a:accounts){   

            AccountShare caSharingRule = new AccountShare();                   
            caSharingRule.AccountId = a.Id;                   
            caSharingRule.OpportunityAccessLevel = 'Edit';                   
            caSharingRule.CaseAccessLevel = 'Edit';                   
            caSharingRule.AccountAccessLevel = 'Edit';                   
            caSharingRule.UserOrGroupId = a.Customer_Success_Manager__c;                   
            ashareLIST.add(caSharingRule);                       

            if(a.Customer_Success_Manager__c!= null){                 
                AccountTeamMember Teammemberad=new AccountTeamMember();
                Teammemberad.AccountId=a.id;
                Teammemberad.UserId=a.Customer_Success_Manager__c;
                Teammemberad.TeamMemberRole = 'Customer Success Manager';                  
                ATM.add(Teammemberad);          

                  if(!ATM.isEmpty()){              
                      insert ATM;                          
                      insert ashareLIST;                                 
                  }               
            }                
        }                
    }              
}

See APEX Dataloader fail INVALID_FIELD_FOR_INSERT_UPDATE, cannot specify Id in an insert call for the trigger this is called from

Answer 1

This a straightforward case where there is no relationship between the two types of objects inserted. So you can simply move the inserts out of the loop:

public class UpdateCSM {   
    public static void updateTeamMember(List<Account> accounts) {
        AccountShare[] shares = new AccountShare[] {};
        AccountTeamMember[] members = new AccountTeamMember[] {};
        for (Account a : accounts) {   
            shares.add(new AccountShare(
                    AccountId = a.Id,
                    OpportunityAccessLevel = 'Edit',
                    CaseAccessLevel = 'Edit',
                    AccountAccessLevel = 'Edit',
                    UserOrGroupId = a.Customer_Success_Manager__c
                    ));
            if (a.Customer_Success_Manager__c != null) {
                members.add(new AccountTeamMember(
                        AccountId = a.id,
                        UserId = a.Customer_Success_Manager__c,
                        TeamMemberRole = 'Customer Success Manager'
                        ));
            }                
        }              
        insert shares;                          
        insert members;                                               
    }              
}

I've removed the unused variables and also changed the code to use the named parameter syntax that results in less clutter. There is also no need to check for empty lists before doing an insert; insert of an empty list does nothing and does not affect governor limit counts.

PS

Or if you want to go for minimum impact on the DML governor limit:

public class UpdateCSM {   
    public static void updateTeamMember(List<Account> accounts) {
        SObject[] sobs = new SObject[] {};
        for (Account a : accounts) {   
            sobs.add(new AccountShare(
                    AccountId = a.Id,
                    OpportunityAccessLevel = 'Edit',
                    CaseAccessLevel = 'Edit',
                    AccountAccessLevel = 'Edit',
                    UserOrGroupId = a.Customer_Success_Manager__c
                    ));
        }
        for (Account a : accounts) {  
            if (a.Customer_Success_Manager__c != null) {
                sobs.add(new AccountTeamMember(
                        AccountId = a.id,
                        UserId = a.Customer_Success_Manager__c,
                        TeamMemberRole = 'Customer Success Manager'
                        ));
            }                
        }              
        insert sobs;                                                                      
    }              
}

The reason to use two loops in this case is that there is a limit on the type transitions within the items in generic SObject collection (otherwise a "System.TypeException: Cannot have more than 10 chunks in a single operation." exception results) so creating the entries with alternating types would make the insert fail for more than 5 Accounts (assuming both objects were always added).