A more efficient way to add query data to a Map

Question

One of my old triggers is beginning to cause Apex CPU Time Limit errors, I was fairly close to this limit when the trigger was first made but since it worked perfectly for so long it was neglected.

The first thing it does it build a Map:

Map<String,Id> comparisonMap = new Map<String,Id>();
for(Lead l : [Select Id, Company, Postcode__c, Landline_Phone__c FROM Lead WHERE isConverted = FALSE AND Check_Lead__c = TRUE]){

    IF(l.Company != null){
        comparisonMap.put(l.Company.toLowerCase().replace(' ','').replace('-',''), l.Id);
    }
    IF(l.Postcode__c != null){
        comparisonMap.put(l.Postcode__c.replace(' ',''), l.Id);
    }
    IF(l.Landline_Phone__c != null){
        comparisonMap.put(l.Landline_Phone__c.replace(' ',''), l.Id);
    }

}

Which is then added to a set for later use:

Set<String> comparisonSet = new Set<String>();
comparisonSet.addAll(comparisonMap.keySet());

Is there a more efficient way of doing this? I think the .toLowerCase() and .replace methods on line 5 are quite taxing on the CPU Time:

comparisonMap.put(l.Company.toLowerCase().replace(' ','').replace('-',''), l.Id);

Any help would be greatly appreciated!

Answer 1

There's no shortcut to what you're doing, but here's a few probable optimizations you can perform.

Use Three Maps

Maps are much faster than manually searching lists (at least 10 times faster on average), partly because they're internally optimized to be faster than linear search algorithms. However, they do not scale as well as lists do. Maintaining three smaller maps should yield better performance.

Use String.replaceAll

Function calls are technically more expensive than simple regexp patterns, so it makes sense to reduce the number of calls you need to make:

l.Company.toLowerCase().replaceAll('[- ]','')

Use Set.clone() When Possible

If you're still using just one map, do this:

Set<String> comparisonSet = comparisonMap.keySet().clone();

This is faster than constructing a new object and using addAll. If you go with three maps, however, you'll still need to use addAll twice to get all the strings in your master set.

References Are Expensive

Every time you need to use a . to deference something, it's costing you CPU time. I'm not sure when the balance between when to switch between direct references to caching variables (because lines are also expensive), but you might try it out:

for(Lead l...) {
    Id leadId = l.Id;
    // Use leadId for the rest of this block
}

Use Internal String Methods, When Possible

You can replace .replace(' ','') with .deleteWhitespace(); this may or may not be more efficient for the Company field, since you're still left with a need to replace one character (the - character), but it's probably better for the other two fields.

Answer 2

I know this has been answered already and this may not help. In your code you are only adding Leads to your map where (Company != null or Postcode__c != null or Landline_Phone__c != null).

I don't know what the rest of your code looks like or if it would help but would it be worth making your initial query more selective?

eg:

for(Lead l : [Select Id, Company, Postcode__c, Landline_Phone__c 
FROM Lead WHERE (isConverted = FALSE AND Check_Lead__c = TRUE])
AND (Company != null or Postcode__c != null or Landline_Phone__c != null))]

Just a thought...