+ Start a Discussion
Mark ErdeljacMark Erdeljac 

Getting Started with Apex Triggers Issue/Question

I was working on the Getting Started with Apex Triggers and my code doesn't seem to be working, could someone please, help me find the error. I have stared at it for awhile and can't seem to figure out where my mistake is, any help that can be provided would be greatly appreciated, thank you in advance for your help and cooperation.

trigger AccountAddressTrigger on Account (before insert, before update) {
    for (Account a : [SELECT BillingPostalCode, ShippingPostalCode FROM Account WHERE BillingPostalCode != null AND Match_Billing_Address__c = TRUE AND Id IN :Trigger.New]) {
        a.ShippingPostalCode = a.BillingPostalCode;
    }
}

And my error is: Challenge not yet complete... here's what's wrong: 
Setting 'Match_Billing_Address__c' to true did not update the records as expected.
Best Answer chosen by Mark Erdeljac
William TranWilliam Tran
CHANGE THIS:

trigger AccountAddressTrigger on Account (before insert, before update) {
    for (Account a : [SELECT BillingPostalCode, ShippingPostalCode FROM Account WHERE BillingPostalCode != null AND Match_Billing_Address__c = TRUE AND Id IN :Trigger.New]) {
        a.ShippingPostalCode = a.BillingPostalCode;
    }
}

TO THIS:

trigger AccountAddressTrigger on Account (before insert, before update) {
     for(Account a : Trigger.New) {
        if(a.Match_Billing_Address__c==true){a.ShippingPostalCode=a.BillingPostalCode;}
    }
}

As a common practice, if your question is answered, please choose 1 best answer. 
But you can give every answer a thumb up if that answer is helpful to you. 

Thanks

All Answers

William TranWilliam Tran
CHANGE THIS:

trigger AccountAddressTrigger on Account (before insert, before update) {
    for (Account a : [SELECT BillingPostalCode, ShippingPostalCode FROM Account WHERE BillingPostalCode != null AND Match_Billing_Address__c = TRUE AND Id IN :Trigger.New]) {
        a.ShippingPostalCode = a.BillingPostalCode;
    }
}

TO THIS:

trigger AccountAddressTrigger on Account (before insert, before update) {
     for(Account a : Trigger.New) {
        if(a.Match_Billing_Address__c==true){a.ShippingPostalCode=a.BillingPostalCode;}
    }
}

As a common practice, if your question is answered, please choose 1 best answer. 
But you can give every answer a thumb up if that answer is helpful to you. 

Thanks
This was selected as the best answer
JLA.ovhJLA.ovh
There is no need to query records in the database. All you need is to update the trigger.new context
 
for(Account a : Trigger.New) {
        if(a.Match_Billing_Address__c && !String.isBlank(a.BillingPostalCode)){
          a.ShippingPostalCode=a.BillingPostalCode;
        }
    }

 
Mark ErdeljacMark Erdeljac
@William Tran and @AdminBooster.com both of your answers work, however this seems to iterate over the entire collection of records which in a production environment would be quite inefficient and may possibly run into Limits, right? Therefore I was looking for a way to query and only update records that met my criteria but Trailhead doesn't seem to recognize my method above, so my actual question was, is what I am doing inappropriate for:
  1. Trailhead's specific request and that is why it isn't being validated?
  2. And/or did I screw up the SOQL that should identify records and that is why trailhead isn't recognizing it? 
William TranWilliam Tran

"iterate over the entire collection of records" --> yes but only for records that are changed so for 99% of the situations, the number of records is 1.

Only when you have batch or other jobs/program like importing data, will you have more than 1.  But if you import you should look into turning your triggers off otherwise it will be SLOW.

Your code does not buy you anything except errors and headaches since your are trying to recreate what you already have by using the IN :Trigger.New clause.

Thx.
sfdc_ninjasfdc_ninja
@William, while I agree with the code you have given, I could not disagree more with the advice you are giving.  All triggers should be optimized and bulkified to work with 1 or many records.  

"yes but only for records that are changed so for 99% of the situations, the number of records is 1." - So your saying its ok for it to crash that 1% of the time?  I totally disagree.  Also who is to say that 99% of the time its a single record?  Different orgs operate differently.  Some orgs do mass uploads and updates daily.  

"But if you import you should look into turning your triggers off otherwise it will be SLOW." - This is just not good advice.  It is not at all best practice to deactivating triggers for bulk operations.  What is the point of having triggers if you have to turn them off for bulk operatons?  Your triggers should be bulkified so whether you are updating 1 or 10,000 records should not make a difference.  If you need to deactivate your triggers to do bulk operations, then you have poorly written triggers.

@Mark, again, the code that @william provided is sound and works.  I just wanted to make sure you didn't think that you should be deactivating triggers to do bulk operations.  That is not at all recomended or best practice.  
 
shashikant pandeyshashikant pandey
trigger AccountAddressTrigger on Account ( before insert, before update) {
    for ( Account a : Trigger.New ){
        if( a . Match_Billing_Address__c == true) {
            a . ShippingPostalCode =  a . BillingPostalCode;
        }

        
    }
}
Jason FungJason Fung
@Mark Erdeljac 
Iterating through the Account object is needed because this trigger happens BEFORE INSERT and BEFORE UPDATE. We don't know which record we should pinpoint and so in order to check if any Account record has a Billing Postal Code and the 'Match_Billing_Address__c' is set as true, you will need to scan through all records in your Account object. In terms of performance, it will definitely slow down performance but that's outside the scope of this trailhead question. The purpose here is to learn how to write a simple trigger function. 

@William Tan
You forgot to check if the BillingPostalCode is empty. Below is what I think the condition in the if statement should look like:
if (a.BillingPostalCode != null && a.Match_Billing_Address__c == true) 
     
shashikant pandeyshashikant pandey
@Jason Fung 
If you have some BillingPastalCode in the Acoount, then you can write the code without checking the BillingPastalCode.
Your code also right you can check both BillingPastalCode and  Match_Billing_Address__c 

Thanks
Mohsin Wadee 17Mohsin Wadee 17
@Jason Fung, I actually disagree with you. To my mind this specific Trailhead module's checking is wrong. This should be the optimal code and should have worked.
trigger AccountAddressTrigger on Account (before insert, before update) {
    
    for (Account a : [SELECT Id, ShippingPostalCode, BillingPostalCode, Match_Billing_Address__c 
                                    FROM Account 
                                    WHERE Id IN :Trigger.New 
                                    AND Match_Billing_Address__c = true]) {
                                        
        a.ShippingPostalCode = a.BillingPostalCode;
    }
}
Salesforce needs to correct this.