0

My requirement is very simple. Whenever a contact filed (Building_Number__c) is updated, update the related account field (block_number__c). I have written code below code for that. My questions are

  1. Is it the best way to do as below?
  2. Are there any other ways (through Apex code itself) to get this done (even if not better ways)

Code:

trigger updateAccount on Contact (before update) {

    List<Account> actList=new List<Account>();

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

    for(Contact con:Trigger.new){
        if(con.building_number__c!=null && con.building_number__c!=Trigger.oldMap.get(con.id).building_number__c){
          actId.add(con.AccountId);  
        }
    }   
    actList=[select id,name,block_number__c from Account where id in:actId ];
    for(Contact con:Trigger.new){
        for(Account acct:actList){
            acct.block_number__c=con.Building_Number__c;
    }
    }
    update actList;
}
  • This might be a better fit over at codereview. Code looks bulkified, but the nested loops towards the end are probably not going to do what you want (because you have every contact updating every account that you queried). – Derek F Jan 6 at 18:44
  • 1
    This requirement doesn't make a lot of sense to me. I would expect the Building Number to be stored on the Account and shown at the Contact level via formula field, since the relationship is one Account to many Contacts. Regardless, this code will behave incorrectly if multiple Contacts are updated from different Accounts. – David Reed Jan 6 at 18:44
4

You could do this using a Process Builder (the downside is this uses more CPU so you could later hit CPU limit issues if you have a lot of other processing going on when contacts are updated).

If you stick with this trigger-based approach note that you are doing far more iteration than needed and there's no need to query the account instances.

I recommend an approach like:

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

for (Contact contact : Trigger.new) {
    if (contact.building_number__c != null && contact.building_number__c != Trigger.oldMap.get(contact.id).building_number__c) {
      updatedContacts.add(contact.Id);  
    }
}

Map<Id, Account> accountsForUpdate = new Map<Id, Account>();

for (Id id : updatedContacts) {
    Contact contact = Trigger.newMap.get(id);

     if (!accountsForUpdate.containsKey(contact.AccountId)) {
         accountsForUpdate.put(contact.AccountId, new Account(Id = contact.AccountId,
             block_number__c = contact.Building_Number__c));
     }
}

update accountsForUpdate.values();

Here I create an in-memory version of the Account (using new Account constructor) and ensure it will update the existing account with a new block number by setting its ID and the block number.

Note that this rather arbitrarily selects the first processed contact related to an Account instance to provide the block number. If multiple different contacts relate to the same account only one of them will be used to update it.

  • 3
    I think that explaining about how the new Account(Id = cont.AccountId) works (why we can use this pattern instead of querying for records) would be a good addition to this answer. – Derek F Jan 6 at 18:47
  • 1
    Also note the business logic described in the OP, when this trigger processes a bulk of Contacts, will update the Account block number based on the first Contact building number processed for each Account, which sounds quite arbitrary. – Ideasforce Jan 6 at 18:58

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.