0

The trigger below is designed to loop through a list of custom objects (VIP_Type__c), that have a master-detail relationship to an Account record, and compile a list of unique values from the VIP_Type__c picklist field of the VIP Type object. It will then write those values to a custom text area field on the Account called VIP_Types__c, separate by colons.

I am having 2 issues that I can't figure out how to resolve.

  1. I need to overwrite the VIP_Types__c field on the Account with the compiled list, but my trigger just appends to any values already there. Essentially, it should create a new list each time and overwrite the Account field.

  2. When the VIP_Types__c field is empty on the Account, my For loop on line 31 throws a null pointer exception:

UpdateAcctVIP: execution of AfterInsert caused by: System.NullPointerException: Attempt to de-reference a null object Trigger.UpdateAcctVIP: line 31, column 1

If the Account field is empty, it should still iterate through the VIP_Type__c child records to compile a list to then write to the Account.

Any ideas on how I can accomplish what I am looking to do?

TRIGGER:

trigger UpdateAcctVIP on VIP_Type__c (after insert, after update, after delete){
        
        Set<Id> acctIds = new Set<ID>();
        
        // Get all the Account Ids in the Set
        if (Trigger.isDelete)
        {
            for(VIP_Type__c vip : Trigger.old){
                acctIds.add(vip.Account__c);
            }

        }
        else
        {
            for(VIP_Type__c vip : Trigger.new){
                acctIds.add(vip.Account__c);
            }
        }
        // QUery the Accounts
        List<Account> acct = new List<Account>();
        // Use the VIP Types to get all the related Types for the Account
        acct = [SELECT Id, VIP_Types__c,(Select VIP_Type__c FROM VIP_Types__r)
                FROM Account
                WHERE Id in :acctIds];
    
    // Iterate over each Account and VIP record
        for(Account a : acct){

               for(VIP_Type__c vip: a.VIP_Types__r){

                if(!a.VIP_Types__c.contains(vip.VIP_Type__c) || a.VIP_Types__c == null){ // Check if the Type is already in the Account Field. if not add it otherwise skip

                    a.VIP_Types__c += vip.VIP_Type__c + ';';

                }
            }

        }
        // Update the Account
        update acct;
    }
2
  • 1
    You accepted my answer on your previous question, but haven't put hardly any of the advice I gave into practice here. I'd encourage you to look through my answer again. Using a Map<Id, Set<String>> is probably the way to go.
    – Derek F
    Sep 2, 2020 at 18:32
  • Sorry. I accepted your previous answer because my initial question was delated to the picklist as a string, which you answered. The items your pointed out were getting too complex for that thread so I started a new one. I will try to re-work this code based upon your previous recommendation, but I suspect I'll still have the same issues I am facing above.
    – J. Neilan
    Sep 2, 2020 at 18:51

1 Answer 1

1

a += b is syntactic sugar for a = a + b
Naturally, if the value of a is null, the expression will fail because any operation on a null value will give you either a Null Pointer Exception or an "argument cannot be null" error.

The other potential issue is your if statement

if(a.VIP_Types__c.contains(vip.VIP_Type__c) || a.VIP_Types__c == null)

NPEs come from when you have null.method() somewhere in your code. If a.VIP_Types__c is null, then that if statement reduces to if(null.contains(thing) || null == null)

Switching the order around so that you do your null check first would cause you to short-circuit the evaluation (i.e. since TRUE || anything evaluates to TRUE, we don't need to evaluate the rest of the statement) and avoid the NPE.

The fix that introduces the least number of lines of new code would be to simply set VIP_Types__c to an empty string in your outer loop.

That is (again, pseudocode)

for(account a :accounts){
    // An empty string is different from a null value
    a.VIP_Types__c = '';

    for(VIP_Type__c vipType){
        a.VIP_Types__c += <some value>;
    }
}

As a bonus, setting VIP_Types__c to an empty string should also take care of the issue of only appending to the existing values.

Once again, you're doing this the hard(er) way. If nothing else, you should be using a Set<String> to collect unique values and String.join() to concatenate the results.

The simplest (though potentially still flawed) way to implement that would be something like

for(account a :accounts){
    Set<String> relatedTypeVals = new Set<String>();

    for(VIP_Type__c vipType){
        relatedTypeVals.add(<some data>);
    }

    a.VIP_Types__c = String.join(new List<String>(relatedTypeVals), ';');
}
1
  • Thanks so much and for your patience! I haven't been coding for the past few years so I'm very rusty, and wasn't even that great to begin with :-). Setting the variable to an empty string did the trick and I will look at using Set<String> to optimize.
    – J. Neilan
    Sep 2, 2020 at 20:16

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .