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
Marmot74Marmot74 

Better Logic needed

Hey Guys, I have the following trigger that basically updates the account table when certain data criteria is met in a custom table. This is a one-many relationship and they relate via AccountId. It errors out with "Too many script statements: 50001". Please help with better logic.

trigger OnLicense_UpdateIsSupported on Licenses__c (after insert, after undelete, after update) {

	Set<String> AccountIDs = new Set<String>();	
	List<Account> updAccounts = new List<Account>();
	
	for(Licenses__c Lic : trigger.new){
		if(!AccountIDs.contains(Lic.Account__c))
			AccountIDs.add(Lic.Account__c);
	}	
		
	List<Account> Accounts = [SELECT ID, IsSupported__c, Is_Customer__c FROM Account WHERE ID IN :AccountIDs];
	List<Licenses__c> lics = [SELECT Account__c, Expire_Date__c FROM Licenses__c WHERE Account__c IN :AccountIDs AND Active__c = true AND IsDeleted = false];
	
	for(Account a : Accounts){
		
		a.Is_Customer__c = false;
		a.IsSupported__c = false;
		
		for(Licenses__c l : lics){			
			if(l.Account__c == a.Id) {
				a.Is_Customer__c = true;
			
				if(l.Expire_Date__c >= Date.Today()) {
					a.IsSupported__c = true; }
			}			
		}
		
		updAccounts.add(a);
	}
			
	update UpdAccounts;

}
 
 


Best Answer chosen by Admin (Salesforce Developers) 
BritishBoyinDCBritishBoyinDC
Yeah - I think you're right - that makes no sense - there's a better example somewhere, but I think the syntax at the end should be as below, so you can retrieve the opps for just the account you are processing
  
Opportunity[] opps = new List< Opportunity>();for (Account eachAcct: accts) {// You can use the get statement from a map to retrieve a list of child objects into a list...  opps = acctOpps.get(eachAcct.Id);// Now you can loop through the opps list for just this account}  //...
 
 
 
 


All Answers
BritishBoyinDCBritishBoyinDC
It looks like that for every account that has a licence that was updated, you are having to looping through every licence for a match with the account, which is very inefficient...
 
I'm not quite sure what the logic is doing, but if you are simply testing for the existence of a licence for an account in the list that has a licence being updated,  a simple map and then using the 'contains' method would remove most of the need for scripting. 
 
If you are needing to check each licence of any single account, you can create maps that create a 1:many relationship in memory, and then for each account, you would only need to look at the licences for that account, which would reduce your need script statements considerably. 
 
Look in the Apex docs for some examples of using maps as in-memory joins... 


Marmot74SFDCMarmot74SFDC
yah thats what I figured. Can you show me how I would combine the following into one MAP? I am still pretty new with this but it's slowly coming along! :)
 
 
	List<Account> Accounts = [SELECT ID, IsSupported__c, Is_Customer__c FROM Account WHERE ID IN :AccountIDs];	List<Licenses__c> lics = [SELECT Account__c, Expire_Date__c FROM Licenses__c WHERE Account__c IN :AccountIDs AND Active__c = true AND IsDeleted = false];
 
 
 


BritishBoyinDCBritishBoyinDC
Take a look at this post - I think it is doing something similar to what you want, so you can see how all the elements work (putting in a map, referencing the key, updating the account object)
 
http://community.salesforce.com/sforce/board/message?board.id=apex&message.id=15586#M15586 


Marmot74Marmot74
It not only has to look for an active licence is also has to see if at least one is supported. It can't just look at just one license because the status can be effected as a whole. I will take a look at the link you sent. All the ones I have looked at so far deal with updated childs records when parent changes. This is the reverse which seems a bit annoying. Basically I am updating the "one" when the "many" changes.


Marmot74Marmot74
Is there a way to create a list from a list with partial values based on a key? I'm used to programming in c# .NET and these list/array objects seem very limit. Hopefully I am wrong...
 
I noted in the below code where if I could get a List that only had the licenses I wanted I would be rolling. Is it possible?
 
	List<Account> Accounts = [SELECT ID, IsSupported__c, Is_Customer__c FROM Account WHERE ID IN :AccountIDs];
	List<Licenses__c> lics = [SELECT Account__c, Expire_Date__c FROM Licenses__c WHERE Account__c IN :AccountIDs AND Active__c = true AND IsDeleted = false];
	
	for(Account a : Accounts){
		
		a.Is_Customer__c = false;
		a.IsSupported__c = false;
		
                  --If I could filter the lics List right here I would be laughing. 

		for(Licenses__c l : lics){			
			if(l.Account__c == a.Id) {
				a.Is_Customer__c = true;
			
				if(l.Expire_Date__c >= Date.Today()) {
					a.IsSupported__c = true; }
			}			
		}
 


BritishBoyinDCBritishBoyinDC
If I understand your requirements, yes.
 
Take a look at this doc - it explains how you can create a map from the List of accounts that is a 1:M map to a second object, in this case the many would be the list of the licences for an Account.
 
Then you can loop through the accounts, and for each account, set a list array to the list of the licences for that account, and update account depending on what is in the array 
 
 


Marmot74Marmot74
thanks for the link. I have a few questions about the code they showed though.
 
Account[] accts = new List<Account>();
Set<Id> acctIDs = new Set<Id>();
// ...
//Imagine some code here that populates the AcctIDs Set.
// ... 
accts = [select Id, Name, BillingAddress, (select Amount, CloseDate from Opportunities) from Account where ID in :acctIDs];


Then we'll build a Map using the Account ID as the key and a List Collection of Opportunity records as the values.

Map<Id, Opportunity []> acctOpps = new Map<Id, Opportunity[]>();
for (Account eachAcct: accts) {
  acctOpps.put(eachAcct.Id, eachAcct.Opportunities);
}


Now, when you use loops to read through the Map, it is all in memory. This is far more efficient than retrieving the data one account at a time:

Opportunity[] opps = new List< Opportunity>();
for (Account eachAcct: accts) {
  opps = [select Id, Amount, CloseDate from Opps where Id = :eachAcct.Id];
  //...
  //code here whatever data manipulation you are attempting.
}
 
In that code they create a map but never reference it? Am I missing something?
 
Alsio in this line
 
opps = [select Id, Amount, CloseDate from Opps where Id = :eachAcct.Id];
 
They reference an object called "Opps", is that a typo?


BritishBoyinDCBritishBoyinDC
Yeah - I think you're right - that makes no sense - there's a better example somewhere, but I think the syntax at the end should be as below, so you can retrieve the opps for just the account you are processing
  
Opportunity[] opps = new List< Opportunity>();for (Account eachAcct: accts) {// You can use the get statement from a map to retrieve a list of child objects into a list...  opps = acctOpps.get(eachAcct.Id);// Now you can loop through the opps list for just this account}  //...
 
 
 
 
This was selected as the best answer
Marmot74Marmot74
Thanks a lot for your help. I got this going nicely now. Here is what I ended up with.
 
trigger OnLicense_UpdateIsSupported on Licenses__c (after insert, after undelete, after update) {

	Set<String> AccountIDs = new Set<String>();	
	
	List<Account> updAccounts = new List<Account>();
	
	for(Licenses__c Lic : trigger.new){
		if(!AccountIDs.contains(Lic.Account__c))
			AccountIDs.add(Lic.Account__c);
	}	
		
	Account[] Accounts = new List<Account>();
	
	Accounts = [SELECT Id, IsSupported__c, Is_Customer__c, (SELECT Id, Account__c, Expire_Date__c FROM Licenses__r WHERE Active__c = true AND IsDeleted = false) FROM Account WHERE Id IN :AccountIDs];

	Map<Id, Licenses__c []> AcctLics = new Map<Id, Licenses__c[]>();
	
	for (Account a :Accounts) {
  		AcctLics.put(a.Id, a.Licenses__r);
	}

	Licenses__c[] Lics = new List<Licenses__c>();
	
	for (Account a :Accounts) {		
  		Lics = AcctLics.get(a.Id);
  		  		
  		a.Is_Customer__c = false;
  		a.IsSupported__c = false;
  		
  		for(Licenses__c l :Lics){
  			a.Is_Customer__c = true;

			if(l.Expire_Date__c >= Date.Today()) {
				a.IsSupported__c = true; }  			
  		}
  		
  		updAccounts.add(a);
	}
			
	update updAccounts;
}
 


BritishBoyinDCBritishBoyinDC
Great - took me a while to understand how maps really worked, but once I worked out how maps worked, I found I used them in almost every trigger/class I wrote...so I think you're in good shape now!