2

I have one trigger which is working exactly how i wanted it to be but the problem is it is not bulkified can somebody help with that

Sales order is child object and Target_V2__c is parent, based on the sales order information automatically parent record will be selected

for (Sales_Order__c soo : trigger.new){
        for(Target_V2__c Target : [SELECT id,name,user__c,FY__c,Quarter_Formula__c,
                                       Practice__c,Practice_Region__c 
                                   FROM Target_V2__c 
                                   WHERE Practice_Region__c =: soo.Practice_Region__c 
                                       AND FY__c =:soo.Year__c 
                                       AND Quarter_Formula__c =: soo.Quarter__c 
                                       AND  Practice__c =: soo.Practicee__c]){
            soo.Target_Practice__c = Target.id;        
        }

    } 

And i know If i query outside for loop then it will be bulkified but the problem here is i put that query outside then how will i compare and get the particular record which i want to access from another object(Target_V2__c) record.

  • Basicly by querying all the probably matches beforehand, and filtering in apex. Can be a bit more complex with more complex where clauses as in your example. Having some more information on your datamodel and conditions may help in for instance. – Samuel De Rycke Jun 30 '14 at 9:43
  • 1
    So, if the sales order Region__c, Year__c, Quarter__c, and Practicee__c fields all match the corresponding fields in the Target_V2__c object, is there really always only a single Target_V2__c record? – pchittum Jun 30 '14 at 9:54
  • possible duplicate of General trigger bulkification - best practices – Christian Deckert Aug 21 '14 at 18:20
1

I would probably suggest a field that concatenated all of the fields in question in Target_V2__c into a Text value in each object, and use that as a key in a map that you populate from your query to perform matching between records in each object. I've done this before (albeit not with four different fields).

You could do this as a text formula field that did the concatenation.

Let's say I did that and called it Composite_Field__c in each object. For Target_V2__c it looked like this:

Practice_Region__c & FY__c & Quarter_Formula__c & Practice__c

And in Sales_Order__c like this:

Practice_Region__c & Year__c & Quarter__c & Practicee__c

(Note: on picklists you'll need to use the TEXT() on any non-text fields for this formula to work, including picklists.)

I can then have my trigger work like so:

Set<String> soCompositeKeys = new Set<String>();
Map<String,Id> targetIdMap = new Map<String,Id>();

//collect keys into a set for querying
for (Sales_Order__c soo : trigger.new){
  soCompositeKeys.add(soo.Composite_Field__c);
} 

//get Target records and stick in map
for(Target_V2__c Target : [SELECT id,Composite_Field__c 
                           FROM Target_V2__c 
                           WHERE Composite_Field__c in: soCompsiteKeys]){
    targetIdMap.put(Target.Composite_Field__c,Target.id);        
}

//assign Target_Practice rel field values based on matching composite field values
for (Sales_Order__c soo : trigger.new){
  soo.Target_Practice__c = targetIdMap.get(soo.Composite_Field__c);
} 

A few thoughts:

  • As per my comment on the question, I worry about the uniqueness of the composite of these four fields. Without that, this whole thing falls apart, so you'd likely want to add some business logic to enforce that, and write a solid round of Apex tests that you run regularly to ensure that this doesn't get broken.
  • I reduced the number of fields in the query as you really only need the two.
  • To improve query performance (provided this formula meets the criteria), you could contact support and ask them to add a custom index to the Composite_Field__c fields.
| improve this answer | |
0

Assuming the point that Peter highlighted in his comment above isn't an issue, this should do the trick:

Set<String> practiceRegions = new Set<String>();
Set<String> fiscalYears = new Set<String>();
Set<String> quarters = new Set<String>();
Set<String> practices = new Set<String>();

for (Sales_Order__c soo : trigger.New) {

    practiceRegions.add(soo.Practice_Region__c);
    fiscalYears.add(soo.Year__c);
    quarters.add(soo.Quarter__c);
    practices.add(soo.Practicee__c);
}

Map<Id, Target_V2__c> relatedTargetMap = new Map<Id, Target_V2__c>([
    SELECT 
        Id,
        Name,
        User__c,
        FY__c,
        Quarter_Formula__c,
        Practice__c,
        Practice_Region__c 
    FROM 
        Target_V2__c 
    WHERE 
        Practice_Region__c IN : practiceRegions
    AND 
        FY__c IN : fiscalYears 
    AND 
        Quarter_Formula__c IN : quarters  
    AND 
        Practice__c IN : practices 
]);

for (Sales_Order__c soo : trigger.New) {

    for (Target_V2__c tar : relatedTargetMap.values()) {

        if (    tar.Practice_Region__c  == soo.Practice_Region__c   &&
                tar.FY__c               == soo.Year__c              &&
                tar.Quarter_Formula__c  == soo.Quarter__c           &&
                tar.Practice__c         == soo.Practicee__c         ) {

            soo.Target_Practice__c = tar.id; 
        }
    }
}
| improve this answer | |
0

Double nested loops over lists of SObjects like this:

for (Sales_Order__c soo : trigger.New) {
    for (Target_V2__c tar : relatedTargetMap.values()) {

result in performance that degrades exponentially as the sizes of the lists grow that can sometimes run you into governor limits. Using maps instead - by creating your own map keys - allows you to get back to linear performance.

For this example that would mean code that looks more like this:

public class MyClass {

    public static void updateTargetPractice(Sales_Order__c[] newSalesOrders) {

        Set<String> practiceRegions = new Set<String>();
        Set<Integer> years = new Set<Integer>();
        Set<Integer> quarters = new Set<Integer>();
        Set<Id> practicees = new Set<Id>();
        for (Sales_Order__c so : newSalesOrders) {
            practiceRegions.add(so.Practice_Region__c);
            years.add(so.Year__c);
            quarters.add(so.Quarter__c);
            practicees.add(so.Practicee__c;
        }

        Map<String, Target_V2__c> m = new Map<String, Target_V2__c>();
        for (Target_V2__c t : [
                select Id, Name, User__c, FY__c, Quarter_Formula__c,
                        Practice__c, Practice_Region__c 
                from Target_V2__c
                where Target_V2__c in :practiceRegions
                and FY__c in :years
                and Quarter_Formula__c in :quarters
                and Practice__c in :practicees
                ]) {
            m.put(key(t), t);
        }

        for (Sales_Order__c so : newSalesOrders) {
            Target_V2__c t = m.get(key(so));
            if (t != null) {
                so.Target_Practice__c = t.Id;
            }
        }
    }

    private static String key(Target_V2__c t) {
        return t.Practice_Region__c + '|' + t.Year__c + '|' + t.Quarter__c
                + '|' + t.Practicee__c;
    }

    private static String key(Sales_Order__c so) {
        return so.Practice_Region__c + '|' + so.FY__c + '|' + so.Quarter_Formula__c 
                + '|' + so.Practice__c;
    }
}
| improve this answer | |
  • Nice approach Keith – Davin Casey Jun 30 '14 at 12:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.