+ Start a Discussion
ScottBScottB 

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;
            							}
            						}

            				}
            
        }
        
    }

 
Best Answer chosen by ScottB
UC InnovationUC Innovation
Hi Scott,

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

UC InnovationUC Innovation
Hi Scott,

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
This was selected as the best answer
UC InnovationUC Innovation
Also dont forget to mark best answer if my solution helped solve your issue(s) so that others can benefit from it as well.
ScottBScottB
Thanks very much for taking so much time to rewrite the code and comment it. Very appreciated!