I'm been tasked w/ updating another Developer's code that was running up against governor limits (specifically the Apex CPU limit).
The code in question is fired via the before update trigger on the default contact object and is used to assign an account (if none exist) based on the contact's company name (set in a custom field).
The first part loops through the contacts and adds them to a list if they don't have an existing account Id while also building the account names set which is later used in an SOQL query:
list<Contact> newContacts = (list<Contact>)Trigger.new;
Set<String> mailingCountries = new Set<String>();
list<User> currentUser = new list<User>();
Set<String> accountNames = new Set<String>();
list<String> excludedAccountRecordTypes = label.Excluded_Account_record_Types.split(','); // Used to limit the query
list<Contact> contactsTobeMapped = new list<Contact>();
for(Contact newCon:newContacts) {
if(newCon.AccountId==null) {
contactsTobeMapped.add( newCon );
if( newCon.Temp_Company_Name__c!=null )
accountNames.add( '%'+newCon.Temp_Company_Name__c+'%' );
if( newCon.mailingCountryCode!=null )
mailingCountries.add( newCon.mailingCountryCode );
}
}
The next part executes the SOQL query (assuming there are company names in the account names set):
if( !accountNames.isEmpty() ) {
list<Account> accounts = new list<Account>();
String accountsQuery = ' SELECT Id, Name, recordtype.name, createdBy.Username, BillingCountry, BillingCountryCode FROM Account '
+ ' WHERE RecordType.Name NOT in:excludedAccountRecordTypes '
+ ' AND Name like :accountNames ';
if(!mailingCountries.isEmpty()) {
accountsQuery += ' AND BillingCountryCode!=null AND BillingCountryCode in:mailingCountries ';
}
accountsQuery +=' LIMIT '+50; // In order to prevent govorner limit errors
accounts = Database.query( accountsQuery );
///...
The final part loops through the returned accounts and inside of that loop each of the contacts and attempts to find a match (partial or exact) between the company name (on the contact) and the account name (and updates the contact account Id if necessary):
for( Account acct: accounts ) {
for( Contact newCon: contactsTobeMapped ) {
if (newCon.AccountId != null) {continue;} // Prevent reassigning accounts to contacts after they've already been assigned
boolean accountNameExactlyMatched = newCon.Temp_Company_Name__c!=null && newCon.Temp_Company_Name__c == acct.Name;
boolean countryNullOrMatched = ( ( newCon.mailingCountry!=null && acct.BillingCountry!=null
&& newCon.mailingCountry == acct.BillingCountry ) ||
( newCon.mailingCountry == null || acct.BillingCountry == null ) );
boolean accountNamePartiallyMatched = ( newCon.Temp_Company_Name__c!=null &&
( newCon.Temp_Company_Name__c.containsIgnoreCase( acct.Name ) ||
acct.Name.containsIgnoreCase( newCon.Temp_Company_Name__c ) ) );
if( countryNullOrMatched && ( accountNameExactlyMatched || accountNamePartiallyMatched ) ) {
newCon.AccountId = acct.Id;
}
}
}
}
This all works as expected and brings me to my question. How can I optimize this?
I'm aware that nested for loops aren't great, but short of rewriting how he's performing the match operation, do I have other options?
As written the operation can be quite expensive from CPU perspective. If the triggering group of contacts is large and the returned accounts from the SOQL is fairly big, the amount of time required to operate can quickly escalate.
I thought about moving it over to an async (e.g. batch) execution, but my understanding is that it's better to perform field updates w/i the before trigger (so that you don't need additional DML operations).
Some of things I have done are:
1) applying a limit on the SOQL query for account (in the dev environment this is actually a dynamic variable set via a custom label) 2) added a check at the top of the inner contact loop to see if the contact had been assigned an account Id in a previous iteration (i.e. prevent reassignment)
Any ideas? I feel like there has to be some better way that I'm missing.
Thanks!
contactsTobeMapped
never have an AccountId and you would never run the same record through the loop again after the first time. Never useRecordType.Name
, always useRecordType.DeveloperName
.RecordType.Name
is not necessarily unique, its translated for international users, and it can be changed at any time between environments.DeveloperName
is analogous to an API name. I imagine you could see a performance boost from usingDeveloperName
too.RecordType.DeveloperName
in place ofRecordType.Name
. One question - doesn't the AccountId check in the third code block prevent unnecessary AccountId reassignment. That is a contact will have a company name that matches against several accounts and so be updated multiple times. With the check, once newCon was assigned an AccountId, wouldn't it skip over it in the next loop? (seemed to in my tests but maybe I'm missing something).contactsTobeMapped
in some way so the set to iterate through gets progressively smaller.