Apex Code Development You need to sign in to do that
Don't have an account?
ScottB How to bulkify and improve this code
Hi, this code is functional but I think that it needs to be bulkified and improved. Can anyone provide a rewrite using best practices please? Thanks in advance!
trigger Results on Results__c (before insert, before update) {
for (Results__c res : Trigger.new) {
if (res.Name == '4'){
List<Game__c> game = [SELECT Fourth_Place_Award__c
FROM Game__c WHERE Id = :res.Game__c];
for(Game__c g : game){
If (g.Fourth_Place_Award__c == 0)
{
res.Bubble__c = TRUE;
}
}
}
}
}
The only thing I can see that is problematic with your code here is the use of a SOQL query inside of a for loop. This is a pretty big issue if you wish to bulkify code.
Consider the scenario that you are updating 201 Results__c records that all have the name '4'. You will hit a governor limit on the last record and see an error. For more information conscerning governor limits check this link (https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_gov_limits.htm). Anyway, with that in mind I went ahead and made some changes to your code and added comments to guide you through it. Added some personal touches as well which you may or may not find useful in future codings.
trigger Results on Results__c (before insert, before update) { // Only looking for results that have the name '4' so this list should be // at most the same size as the original trigger list. // To bulkify this trigger we will need to look through // Trigger.New twice. List<Results__c> fourList = new List<Results__c>(); // We want to store the Id of the games we wish to query for // instead of querying for every game. This keeps our SOQL // count to a minimum. Set<Id> gameIdSet = new Set<Id>(); // First look: fill the containers for (Results__c result : Trigger.new) { if (result.Name.equals('4')){ fourList.add(result); gameIdSet.add(result.Game__c); } } // One query instead of many. Governor limits in mind. Map<Id, Game__c> gameMap = new Map<Id, Game__c>([SELECT Fourth_Place_Award__c FROM Game__c WHERE Id IN :gameIdSet]) // Second look: record results. for (Results__c result : fourList) { if (gameMap.get(result.Game__c).Fourth_Place_Award__c == 0) { result.Bubble__c = TRUE; } } }Hope this helps! Good luck and if you have any further questions feel free to ask away.
AM
All Answers
The only thing I can see that is problematic with your code here is the use of a SOQL query inside of a for loop. This is a pretty big issue if you wish to bulkify code.
Consider the scenario that you are updating 201 Results__c records that all have the name '4'. You will hit a governor limit on the last record and see an error. For more information conscerning governor limits check this link (https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_gov_limits.htm). Anyway, with that in mind I went ahead and made some changes to your code and added comments to guide you through it. Added some personal touches as well which you may or may not find useful in future codings.
trigger Results on Results__c (before insert, before update) { // Only looking for results that have the name '4' so this list should be // at most the same size as the original trigger list. // To bulkify this trigger we will need to look through // Trigger.New twice. List<Results__c> fourList = new List<Results__c>(); // We want to store the Id of the games we wish to query for // instead of querying for every game. This keeps our SOQL // count to a minimum. Set<Id> gameIdSet = new Set<Id>(); // First look: fill the containers for (Results__c result : Trigger.new) { if (result.Name.equals('4')){ fourList.add(result); gameIdSet.add(result.Game__c); } } // One query instead of many. Governor limits in mind. Map<Id, Game__c> gameMap = new Map<Id, Game__c>([SELECT Fourth_Place_Award__c FROM Game__c WHERE Id IN :gameIdSet]) // Second look: record results. for (Results__c result : fourList) { if (gameMap.get(result.Game__c).Fourth_Place_Award__c == 0) { result.Bubble__c = TRUE; } } }Hope this helps! Good luck and if you have any further questions feel free to ask away.
AM