+ Start a Discussion
Rookie DeveloperRookie Developer 

Rookie Trigger

I discovered geocodes could be added to Accounts without needing code but for the sake of learning, could someone provide a newbie with constructive feedback on this trigger? I'd really like to figure this out.

I have two sales reps splitting California for their sales territory. One owns anything above Santa Barbara and one owns everything else. I created a custom object labeled 'Cities' with fields for City__c and Latitude__c, and imported this data for all California cities.

Whenever an Account is created or updated, I want my trigger to query the custom object 'Cities', match the Account BillingCity to the City__c field on my custom object, and evaluate if the corresponding Latitude__c field on my custom object is greater than Santa Barbara's latitude.  

If the latitude is greater than Santa Barbara's, add the Account to a list which will be updated with the Northern Sales Rep as the Owner. If the latitude is not greater than Santa Barbara's, add the Account to a list which will be updated with the Southern Sales Rep as the Owner.

When I test the trigger, I get the dreaded "System.NullPointerException: Attempt to de-reference a null object ()" error which confuses me because I check to see if BillingCity is null and confirmed I have all California cities and latitudes imported into my custom object. I have a feeling the error is somewhere around line 13 where I query my custom object and try to find the corresponding latitude...

Thanks to anyone for taking the time to help me learn! 

Here is my trigger:

trigger CaliSalesTerritory on Account (before insert, before update) {

    List <Account> caliAccts; // A List to hold all California Accounts
    List <Account> norCalCities; // A List to hold all Northern California Accounts
    List <Account> soCalCities; // A List to hold all Southern California Accounts
    
    for (Account acct : Trigger.new) { 
        if (acct.BillingState == 'California' && acct.BillingCity != null) {
            caliAccts.add(acct); 
// For all Accounts being inserted or updated, if the BillingState is "California" and the BillingCity isn't null, add the Account to the caliAccts list    
        }
    } 
   
    for (Account acct2 : caliAccts) {
        Cities__c caliLatitude = [Select Latitude__c FROM Cities__c WHERE City__c = :acct2.BillingCity]; 
 // For all Accounts in the caliAccts list, query the Cities__c object and return the Latitude based on the Account's BillingCity
       
    if (caliLatitude.Latitude__c > 34.4285) {
           norCalCities.add(acct2);
// If the Latitude is greater than Santa Barbara's latitude, add the Account to the norCalCities list            
        }    
            else {
                soCalCities.add(acct2);
// If the Latitude is not greater than Santa Barbara's latitude, add the Account to the soCalCities list                            
            }
    }
           
    for (Account northRepAcct : norCalCities) {
        northRepAcct.OwnerId = '0050f0000097wqI'; //Northern Sales Rep UserId
// Assign all Accounts in the norCalCities list to the Northern Sales Rep
    }  
    update norCalCities;
    
    for (Account southRepAcct : soCalCities) {
        southRepAcct.OwnerId = '0050f000009K2kr'; //Souther Sales Rep UserId
// Assign all Accounts in the soCalCities list to Southern Sales Rep        
    } 
    update soCalCities;
}
Boss CoffeeBoss Coffee
Try initializing your lists first before adding to them.
List <Account> caliAccts = new List<Account>(); // A List to hold all California Accounts 
List <Account> norCalCities = new List<Account>(); // A List to hold all Northern California Accounts 
List <Account> soCalCities = new List<Account>(); // A List to hold all Southern California Accounts
Rookie DeveloperRookie Developer

(facepalm)

that's embarrassing... thanks, Boss Coffee! i'll report back on my rpogress. have a great day

Boss CoffeeBoss Coffee
No problem! Let me know if you need any more help.
Rookie DeveloperRookie Developer
Hi Boss Coffee! Here is my test class:
@istest
public class testCaliSalesTerritory {

    @istest static void createUpdateAccount() {
        Account myAcct      = new Account();
        myAcct.Name         = 'bbCaliTestAcct';
        myAcct.BillingCity  = 'Santa Barbara';
        myAcct.BillingState = 'California';
        insert myAcct;
    }
    
}

When I run this test class, I only get 47% code coverage. It looks like I'm not querying the Cities__c custom object correctly to retrieve the corresponding Latitude__c:
User-added image
Line 16 is syntactically correct I guess but it's not retrieiving the Latitude value to evaluate if it's north or south of Santa Barbara. Here are those lines of code:

if (caliLatitude.Latitude__c > 34.4285) {
     norCalCities.add(acct2);
// If the Latitude is greater than Santa Barbara's latitude, add the Account to the norCalCities list            
}
Any advice? I'm guessing my SOQL statement isn't correct. Thanks for your time!
Rookie DeveloperRookie Developer
or actually it may be the lines of code before with the SOQL query:
for (Account acct2 : caliAccts) {
     Cities__c caliLatitude = [Select Latitude__c FROM Cities__c WHERE City__c = :acct2.BillingCity]; 
 // For all Accounts in the caliAccts list, query the Cities__c object and return the Latitude based on the Account's BillingCity
Boss CoffeeBoss Coffee
In your test class you need to create the Cities__c records as well, otherwise the list will be empty when it reaches that line.

Additionally, a bit unrelated, but it's best to not include queries inside of for loops because you will very quickly hit your governor limits when processing a large number of records.
Rookie DeveloperRookie Developer

(another facepalm)

will do Boss Coffee and thanks for the pro tip regarding queries inside of for loops. i'll let you know how it goes. have a great day and thanks again for your help!

Greg HGreg H
I'm glad you are taking the time to learn a practical example. Good feedback has already been shared.Here is some more:
  • Instead of checking for null on acct.BillingCity try using the String.isNotBlank(acct.BillingCity) method. It checks for null, blank and empty ''.
  • Avoid using actual hard coded Ids in your logic. It is easy to do but it can cause deployment issues if you've coded for a sandbox Id that is not the same in your production Org.
Good luck with your development.
-greg
Rookie DeveloperRookie Developer
thanks greg! i feel like i've learned so much already. i'll incorporate your tips and report back on my progress. have a great day!