+ Start a Discussion
Big_BBig_B 

Nested for-loops

Hi Everyone!

 

I'm pretty new to Apex and am having an issue with bulkifying my Asset trigger code below.  Basically, I need to update account records based on information on the related asset records.  The code takes the Asset(s) from the trigger, creates a set for all the related accounts then uses that set in a SOQL query to pull a list of accounts and then pull a list of all active assets related to those accounts - which could include more asset records than the trigger asset(s).

 

I then have a for-loop to iterate through the accounts.  Inside that for-loop I then have another for-loop to iterate through the assets. If the account id in the asset for-loop == account in the account for-loop, use the asset info to update the account info.

 

Everything works fine until I do an update to the Asset object using data loader when there are a lot of records.  It successfully updates about 20 records and then the remaining records of the 200 record batch fail.  The error I get is:

 

'AssetTrigger: System.LimitException: Too many script statements: 200001'

 

I assume it is because I am iterating through ALL the asset records for each account id to look for a match that brings the statement count past the limit but I can't figure out a more efficient way to do it.  Is there a way to set up the asset for-loop to only iterate through the asset records where the account Id matches?  Could it be something else?  200000 statements seems REALLY high.

 

Any assistance would appreciated.  Don't be afraid to provide suggestions for improving any of the code - I can take it!

 

 

 

public with sharing class UpdateAcctFromAsset {

	public void updateAcctCustInfo(List<Asset> trigAssets)
	{	
		String sProdFam_TA = 'Recruitment';
		String sProdFam_TM = 'Talent Management';		
		String sProdFam_WCAD = 'Workforce Compliance';
		String sProdFam_VMS = 'Vendor Management';
		String sProdFam_LMS = 'Learning Management';
		String sProdFam_AQ = 'Aquire';
		String sProdFam_PRI = 'PRI';
		String sProdFam_Tablet = 'Tablet Application';
		
		//Get account id's from all Assets
		Set<Id> accountIds = new Set<Id>();
		for(Asset a: trigAssets){
			accountIds.add(a.AccountId);						
		}		
		//Get list of customer accounts so we can set the new values
		List<Account> custAccts = new List<Account>([Select Id, Type, TA_Client__c, TM_Client__c, WCAD_Client__c, 														LMS_Client__c, PRI_Client__c, VMS_Client__c, Aquire_Client__c, Tablet_Client__c
								from Account 
								where id IN :accountIds]);
														
		List<Asset> acctAssets	= new List<Asset>([Select Id, AccountId, Product_Family__c, System_Status__c 
								from Asset
								where AccountId IN :accountIds 
								AND (NOT System_Status__c = 'Inactive') 														AND (NOT System_Status__c = 'Cancelled')]);											
		
		//Cycle through accounts and update values		
		for(Account acct: custAccts){
			acct.TA_Client__c = false;
			acct.TM_Client__c = false;
			acct.WCAD_Client__c = false;
			acct.LMS_Client__c = false;
			acct.PRI_Client__c = false;
			acct.VMS_Client__c = false;
			acct.Aquire_Client__c = false;
			acct.Tablet_Client__c = false;
			Boolean bIsCustomer = false;		
			
			//Go through each Asset and update				
			for (Asset ast : acctAssets){
				if (ast.System_Status__c != 'Inactive' && ast.System_Status__c != 'Cancelled' && ast.AccountId == acct.Id){
					If (ast.Product_Family__c == sProdFam_TA){
						acct.TA_Client__c = true;
						bIsCustomer = true;
					}
					If (ast.Product_Family__c == sProdFam_TM){
						acct.TM_Client__c = true;
						bIsCustomer = true;
					}	
					If (ast.Product_Family__c == sProdFam_WCAD){
						acct.WCAD_Client__c = true;
						bIsCustomer = true;
					}
					
					If (ast.Product_Family__c == sProdFam_VMS){
						acct.VMS_Client__c = true;
						bIsCustomer = true;
					}
						
					If (ast.Product_Family__c == sProdFam_LMS){
						acct.LMS_Client__c = true;
						bIsCustomer = true;
					}
						
					If (ast.Product_Family__c == sProdFam_PRI){
						acct.PRI_Client__c = true;
						bIsCustomer = true;
					}
					
					If (ast.Product_Family__c == sProdFam_AQ){
						acct.Aquire_Client__c = true;
						bIsCustomer = true;
					}
						
					If (ast.Product_Family__c == sProdFam_Tablet){
						acct.Tablet_Client__c = true;
						bIsCustomer = true;
					}
				}									
			}
			//Update Account type status
			if (acct.Type != 'Partner' && bIsCustomer == true){
				acct.Type = 'Customer';			
			}
			if (acct.Type != 'Partner' && bIsCustomer == false){
				acct.Type = 'Prospect';
			}
		}					
		update custAccts;			
	}

 

Aar3538Aar3538

Hello Big B,

 

After looking over your code I do agree with your logic that its covering all assets for each account and that is causing you to hit the Script Statement Limit.  One solution for this would be to create a map of Account Ids to a list of Assets  and then after you query for all of the assets, loop through the assets once as shown below:

 

List<Asset> acctAssets= new List<Asset>([Select Id, AccountId, Product_Family__c, System_Status__c from Asset

where AccountId IN :accountIds AND (NOT System_Status__c = 'Inactive')AND (NOT System_Status__c = 'Cancelled')]);

 

transient Map <Id, List<Asset>> assetsToAccountMap = newMap<String,List<Asset>>();

 

for (Asset a: acctAssets){

 List<Asset> tempAsset = new List<Asset>();

 //If the map already contains this account, you want to append to the list.

  if (assetsToAccountMap.containsKey(a.AccountId)){

 tempAsset = assetsToAccountMap.get(a.AccountId);
}

tempAsset.add(a);

assetsToAccountMap.put(a.AccountId, tempAsset);

}

 

 

This way you have a map of a list of the assets per account.  So when you are looping through your accounts you can do:

 

for(Account acct: custAccts){

  if (assetsToAccountMap.containsKey(acct.Id)){

//assetsForThisAccount will contain all assets only for the current Account

transient List<Asset> assetsForThisAccount = assetsToAccountMap.get(acct.Id);

}

 

}

 

 

Please note that this map will be fairly large as a result (A map of a list of Assets) and you may be trading too many script statements in for Heap Size errors.  As a result of this, instead of creating a list of Asset it might be best to create a list of Strings or something smaller to hold System Status and Product Family (instead of the whole Asset object).

 

Also to cut down on heap size issues, always be sure to prefix any variables that wont be shown on visualforce pages with the transient keyword.  Minor nagging also would be to have variables like sProdFam_TA be static and final if its applicable.

 

I really hope this helps!

 

Edit:  Also as a reminder while it is in your best interest to make the trigger the most efficient it can possibily be, sometimes its an impossible goal to make it work for batch size of 200.  Just to make sure you were aware, dataloader allows you to set the batch size (from 200 down to 1 at a time).  While not ideal, sometimes records have to come in at a lower batch size in order to work within the confines of the limits.

Big_BBig_B

Thanks for the response, Aar3538!  You have introduced me to some new and creative concepts that I still need to wrap my head around but from what I can tell your idea may do the trick.  I have a couple other projects I need to attend to now but I will test your concept when I get some time and let you know.

 

Also, thanks for the heap size and data loader tips - very helpful!