1

REQUIREMENT: Need to move the query outside of the FOR loop in Opportunity trigger.

PROBLEM: After moving query outside of the FOR loop, I receive an error when trying to save a new Opportunity record (when it is triggered).

Original Code (no issues):

trigger myTrigger on Opportunity (before insert) {

    Set<Id> parentAccountIds = new Set<Id>();

    for(Opportunity opp : Trigger.new) {
        parentAccountIds.add(opp.AccountId);

        //query fields on parent accounts and map them by Account Id
        Map<Id, Account> parentAccountsById = new Map<Id, Account>([
            select Id, CustomField__c
            from Account
            where Id in :parentAccountIds
        ]);

        if(String.isNotBlank(opp.AccountId)) {

            Date CustomField = parentAccountsById.get(opp.AccountId).CustomField__c;

            // do stuff

        }
    }
}

Code after moving the query, has error (see below):

trigger myTrigger on Opportunity (before insert) {

    Set<Id> parentAccountIds = new Set<Id>();

    //query fields on parent accounts and map them by Account Id
    Map<Id, Account> parentAccountsById = new Map<Id, Account>([
        select Id, CustomField__c
        from Account
        where Id in :parentAccountIds
    ]);

    for(Opportunity opp : Trigger.new) {

        parentAccountIds.add(opp.AccountId);

        if(String.isNotBlank(opp.AccountId)) {
            Date CustomField = parentAccountsById.get(opp.AccountId).CustomField__c;

            // do stuff

        }
    }
}

ERROR:

myTrigger: execution of BeforeInsert caused by: System.NullPointerException: Attempt to de-reference a null object Trigger.myTrigger: line 22, column 1

LINE 22 refers TO:

Date CustomField = parentAccountsById.get(opp.AccountId).CustomField__c;
3

The answer by the OP does solve the problem, but I'd like to go into more detail about why it works (and why the initial attempt didn't work).

Let's take the bottom-most loop over Trigger.new

for(Opportunity opp : Trigger.new) {

    if(String.isNotBlank(opp.AccountId)) {
        Date CustomField = parentAccountsById.get(opp.AccountId).CustomField__c;

        // do stuff

    }
}

The if statement ensures that our Opportunity does indeed have an AccountId, but we can still run into a Null Pointer Exception (NPE). Without performing some sort of null check on the map (or result), parentAccountsById.get(opp.AccountId) can return null.

null.<anything> results in an NPE, because null has no properties, variables, or methods.

Looking at OP's full first attempt (with my own comments):

trigger myTrigger on Opportunity (before insert) {

    // This line both declares and initializes parentAccountIds
    // This allows us to use the standard set methods, but the set is empty
    Set<Id> parentAccountIds = new Set<Id>();

    // In this query, we haven't put anything into parentAccountIds
    // This query will thus return 0 rows, but still give us our initialization
    //   for the map parentAccountsById.
    // We can call any standard method of the Map class, but the map will contain nothing
    Map<Id, Account> parentAccountsById = new Map<Id, Account>([
        select Id, CustomField__c
        from Account
        where Id in :parentAccountIds
    ]);

    for(Opportunity opp : Trigger.new) {

        // We're adding accountIds to the parentAccountIds set _after_ running
        //   the query
        parentAccountIds.add(opp.AccountId);

        if(String.isNotBlank(opp.AccountId)) {
            // The opp's AccountId is not blank, but because we have nothing in
            //   parentAccountsById, we end up trying to call null.CustomField__c
            //  which results in an NPE
            Date CustomField = parentAccountsById.get(opp.AccountId).CustomField__c;

            // do stuff

        }
    }
}

The solution, as illustrated by paulK, is to use the following pattern:

  1. Declare/initialize a collection
  2. Iterate over Trigger.new, and add the Ids to the collection
  3. Perform a query, using the collection of Ids gathered previously
  4. Iterate over Trigger.new again, this time doing the work you want to actually do

Again, the key part of this pattern is that we iterate twice over Trigger.new. We need to gather the Ids we want to use in the query before we execute the query.

  • Nice explanation, Derek! Have an upvote! – paulK Nov 16 '18 at 23:25
1

Ok, found the solution...

trigger myTrigger on Opportunity (before insert) {

    Set<Id> parentAccountIds = new Set<Id>();

    for(Opportunity newOpp : Trigger.new) {
        parentAccountIds.add(newOpp.AccountId);
    }

    //query fields on parent accounts and map them by Account Id
    Map<Id, Account> parentAccountsById = new Map<Id, Account>([
        select Id, CustomField__c
        from Account
        where Id in :parentAccountIds
    ]);

    for(Opportunity opp : Trigger.new) {

        if(String.isNotBlank(opp.AccountId)) {
            Date CustomField = parentAccountsById.get(opp.AccountId).CustomField__c;

            // do stuff

        }
    }
}

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.