mjhjmtu

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;

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.

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