3

How would you rewrite the following trigger to follow best practices re: bulkification, etc?

trigger Order_PopulateGallery on RH_Order__c (before insert, before update){

List<Account> accs = new List<Account>();

for(RH_Order__c ord : Trigger.new){
    if(Trigger.isInsert){
        if(ord.Gallery_Number__c != null){
            accs = [Select Id from Account where Gallery__c = :ord.Gallery_Number__c limit 1];
                if(accs.size() != 0){
                    ord.Gallery__c = accs[0].Id;
                }
                else{
                    ord.addError('  Please enter valid Gallery number.');
                }
        }       
    }
    else{
        if(ord.Gallery_Number__c != null){
            RH_Order__c oldOrd = Trigger.oldMap.get(ord.Id);
            if(oldOrd.Gallery_Number__c != ord.Gallery_Number__c){
                accs = [Select Id from Account where Gallery__c = :ord.Gallery_Number__c limit 1];
                if(accs.size() != 0){
                    ord.Gallery__c = accs[0].Id;
                }
                else{
                    ord.addError(' Please enter valid Gallery number.');
                }
            }       
        }
        else{
            ord.Gallery__c = null;
        }                
    }
}
}
7

First, you should try to follow the DRY pattern; by moving some logic around, we can reduce your code by almost half:

trigger Order_PopulateGallery on RH_Order__c (before insert, before update){

    List<Account> accs = new List<Account>();

    for(RH_Order__c ord : Trigger.new){
        if(ord.Gallery_Number__c == null) {
            ord.Gallery__c = null;
        } else
            if(Trigger.isInsert || Trigger.oldMap.get(ord.Id).Gallery_Number__c != ord.Gallery_Number__c) {
                accs = [Select Id from Account where Gallery__c = :ord.Gallery_Number__c limit 1];
                if(accs.size() != 0){
                    ord.Gallery__c = accs[0].Id;
                }
                else{
                    ord.addError('  Please enter valid Gallery number.');
                }
            }
    }
}

Second, we use the Aggregate-Query-Update pattern to bulkify your trigger:

trigger Order_PopulateGallery on RH_Order__c (before insert, before update){
    Set<String> galleryNumbers = new Set<String>();
    Map<String, Id> galleryIds = new Map<String, Id>();

    // Aggregate
    for(RH_Order__c ord: Trigger.new) {
        if(ord.Gallery_Number__c != null && (Trigger.isInsert || Trigger.oldMap.get(ord.Id).Gallery_Number__c != ord.Gallery_Number__c)) {
            galleryNumbers.add(ord.Gallery_Number__c);
        }
    }

    // Query
    if(!galleryNumbers.isEmpty()) {
        for(Account record: [SELECT Gallery__c FROM Account WHERE Gallery__c IN :galleryNumbers]) {
            galleryIds.put(record.Gallery__c, record.Id);
        }
    }
    // Update
    for(RH_Order__c ord: Trigger.new) {
        if(ord.Gallery_Number__c == null) {
            ord.Gallery__c = null;
        } else
            if(Trigger.isInsert || Trigger.oldMap.get(ord.Id).Gallery_Number__c != ord.Gallery_Number__c) {
                if(galleryIds.get(ord.Gallery_Number__c) == null) {
                    ord.addError('Please enter valid Gallery number.');
                } else {
                    ord.Gallery__c = galleryIds.get(ord.Gallery_Number__c);
                }
            }
    }
}

Note that bulkification almost always results in slightly longer code, simply because of the extra variables that are necessary.

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.