+ Start a Discussion
MrHammyMrHammy 

Need to bulkify

Hi!

I am setting up a class, called via a trigger, to set an email address. I need to go from my custom object Product_Assessment__c to its parent case and then on to the contact, retrieve the email and populate it. I have the class working but it is not bulkified, my loop as 2 SOQL query's in it and  as we all know that's just BAD! I know I need to build up a map to return the correct  email address but am struggling a bit. can someone lend a hand? 
​Thanks for your time!


 
public with sharing class Product_Assessment_set_contact_email {
	
	// These variables store Trigger.oldMap and Trigger.newMap
  Map<Id, Product_Assessment__c> oldpas;
  Map<Id, Product_Assessment__c> newpas;	
	
// This is the constructor
// A map of the old and new records is expected as inputs
  public Product_Assessment_set_contact_email(
    Map<Id, Product_Assessment__c> oldTriggerpas, 
    Map<Id, Product_Assessment__c> newTriggerpas) {
      oldpas = oldTriggerpas;
      newpas = newTriggerpas;
  }
  
  
  public void set_email(){
  

  	for (Product_Assessment__c p : newpas.values())      
       {
       	
       	case con = [SELECT ContactId FROM Case WHERE Id = :p.case__c ];
       	
       	contact em = [select email from contact where id = :con.ContactId];
       	
       	p.email_of_contact__c = em.email;

       }
  }
  

}

 
Best Answer chosen by MrHammy
Abhishek BansalAbhishek Bansal
Hi Hammy,

Please use the below code :
public with sharing class Product_Assessment_set_contact_email {
	// These variables store Trigger.oldMap and Trigger.newMap
  	Map<Id, Product_Assessment__c> oldpas;
  	Map<Id, Product_Assessment__c> newpas;	
	
	// This is the constructor
	// A map of the old and new records is expected as inputs
  	public Product_Assessment_set_contact_email(
    Map<Id, Product_Assessment__c> oldTriggerpas, 
    Map<Id, Product_Assessment__c> newTriggerpas) {
      	oldpas = oldTriggerpas;
      	newpas = newTriggerpas;
  	}
  
  	public void set_email(){
  	
  		Set<Id> caseIds = new Set<Id>();
  		for(Product_Assessment__c p : newpas.values()){
  			if(p.case__c != null){
  				caseIds.add(p.case__c);
  			}
  		}
  		Map<Id,Case> caseMap = new Map<Id,Case>();
  	
  		for(Case ca : [SELECT Contact.Email FROM Case WHERE Id = :caseIds]){
  			if(ca.Contact.Email != null){
  				caseMap.put(ca.Id,ca);
  			}
  		}
	
  		for (Product_Assessment__c p : newpas.values()) {
       	
       		if(caseMap.containsKey(p.case__c)){
       	
       			p.email_of_contact__c = caseMap.get(p.case__c).Contact.Email;
       		}

       	}
  	}
}

Let me know if you have any issue with the above code.

Thanks,
Abhishek

All Answers

Abhishek BansalAbhishek Bansal
Hi Hammy,

Please use the below code :
public with sharing class Product_Assessment_set_contact_email {
	// These variables store Trigger.oldMap and Trigger.newMap
  	Map<Id, Product_Assessment__c> oldpas;
  	Map<Id, Product_Assessment__c> newpas;	
	
	// This is the constructor
	// A map of the old and new records is expected as inputs
  	public Product_Assessment_set_contact_email(
    Map<Id, Product_Assessment__c> oldTriggerpas, 
    Map<Id, Product_Assessment__c> newTriggerpas) {
      	oldpas = oldTriggerpas;
      	newpas = newTriggerpas;
  	}
  
  	public void set_email(){
  	
  		Set<Id> caseIds = new Set<Id>();
  		for(Product_Assessment__c p : newpas.values()){
  			if(p.case__c != null){
  				caseIds.add(p.case__c);
  			}
  		}
  		Map<Id,Case> caseMap = new Map<Id,Case>();
  	
  		for(Case ca : [SELECT Contact.Email FROM Case WHERE Id = :caseIds]){
  			if(ca.Contact.Email != null){
  				caseMap.put(ca.Id,ca);
  			}
  		}
	
  		for (Product_Assessment__c p : newpas.values()) {
       	
       		if(caseMap.containsKey(p.case__c)){
       	
       			p.email_of_contact__c = caseMap.get(p.case__c).Contact.Email;
       		}

       	}
  	}
}

Let me know if you have any issue with the above code.

Thanks,
Abhishek
This was selected as the best answer
JayNicJayNic
Good for you for wanting to bulkify!
Remember: when you want to bulk things up: all you need to do is gather all your criteria in to one place, and query against it. In this case: you just want case ids - so the easiest way it to make a set of case ids, and query for the values on those cases.

Here is my suggestion - this is UNTESTED and probably has syntax errors:
public with sharing class Product_Assessment_set_contact_email {
	
	// These variables store Trigger.oldMap and Trigger.newMap
  Map<Id, Product_Assessment__c> oldpas;
  Map<Id, Product_Assessment__c> newpas;	
	
// This is the constructor
// A map of the old and new records is expected as inputs
  public Product_Assessment_set_contact_email(
    Map<Id, Product_Assessment__c> oldTriggerpas, 
    Map<Id, Product_Assessment__c> newTriggerpas) {
      oldpas = oldTriggerpas;
      newpas = newTriggerpas;
  }
 
 //CASE IDS
 //Returns a new set of Case ids from related Product assesments
 //Find all your criteria: eg: the cases you want
 public set<id> getCaseIds(){
	set<id> ids = new set<id>();
	if(!newpas.values().isEmpty()){
		for (Product_Assessment__c p : newpas.values()){
			if(p.Case__c != null){
				ids.add(p.Case__c);
			}
		}
	}
	return ids;
 }
 
 //CASES
 //Query for cases base don case ids
 //You need to query for all data at once - not in a loop
 private map<id,Case> Cases{
	 get {
		if(Cases == null){
			Cases = new map<id,Case>();
			set<id> ids = getCaseIds();
			if(!ids.isEmpty()){
				//If you construct a map with a query: it will automatically return a map of those record ids to those sobjects
				Cases = new map<id,Case>([SELECT id, Contact.Email FROM Case WHERE Id IN: ids]);
			}
		}
		 return Cases;
	 }
	 set;
 }
 
  
  public void set_email(){
  

  	for (Product_Assessment__c p : newpas.values())      
       {
       	p.email_of_contact__c = p.case__c != null && Cases.get(p.case__c) != null ? p.Contact.Email : null;

       }
  }
  

}

 
JayNicJayNic
@Ketankumar Patel
That will fail - the values were not queried for...
Ketankumar PatelKetankumar Patel
Thanks, JayNic. You're right this will fail in apex trigger. I need to query relationship fields. 

Corrected my logic.
public with sharing class Product_Assessment_set_contact_email {
	
  public Product_Assessment_set_contact_email(Map<Id, Product_Assessment__c> oldTriggerpas, Map<Id, Product_Assessment__c> newTriggerpas)
  {
     //if case is a parent of Product_Assessment_set_contact_email then you can grab field from parent object using lookup relationshipName with __r
     Map<Id,Product_Assessment__c> ProductAssessmentMap = Map<Id,Product_Assessment__c>([Select Id, case__r.contact.email from Product_Assessment__c WHERE Id IN: newTriggerpas WHERE case__c != NULL]);
	 for(Product_Assessment__c p : newTriggerpas)      
       {
         //Your logic
		   if(p.case__c != NULL){
       	      p.email_of_contact__c = ProductAssessmentMap.get(p.Id).case__r.contact.email;
		 }
       }
  }

}