0

I have written this trigger in this can we avoid the second for loop using map.

Trigger Sum on Contact(after insert, after update, after delete, after undelete)
{
    Set<Id> accountIdSet = new Set<Id>();
    List<Account> accList = new List<Account>();
    Map<Id, Decimal> sumMap = new Map<Id, Decimal>();
    If(Trigger.isInsert || Trigger.isUpdate || Trigger.isUndelete)
    {
        for(Contact con : Trigger.new)
        {
            accountIdSet.add(con.accountId);
            if(!sumMap.containsKey(con.accountId))
            {
                sumMap.put(con.accountId, con.Salary__c);
            }
        //  sumMap.get(con.accountId).add(con.Salary__c);
        }
    }
    If(Trigger.isUpdate || Trigger.isDelete)
    {
        for(Contact con : Trigger.old)
        {
            accountIdSet.add(con.accountId);
            if(!sumMap.containsKey(con.accountId))
            {
                sumMap.put(con.accountId, con.Salary__c);
            }
        //  sumMap.get(con.accountId).add(con.Salary__c);
        }
    }   
    for(Account acc : [Select id, ContactsSalarySum__c, (Select id, Salary__c From Contacts) From Account Where Id In =: accountIdSet])
    {
        Decimal sum = 0;
        for(Contact con : acc.contacts)
        {
            sum = sum + con.salary__c;
        }
        acc.ContactsSalarySum__c = sum;
        //acc.ContactsSalarySum__c = sumMap.get(acc.id);
        accList.add(acc);
    }
    update accList;
}
4
  • 1
    Use dlrs for rollups. Don't roll your own, especially if you're not 100% sure what you're doing already. Clicks not code!
    – Adrian Larson
    Dec 16, 2016 at 19:12
  • aside from adrian's remark which is correct; since the trigger is on Contact, you will always need two for loops to update a related parent object : (1) to get Ids of parent Object and (2) to fetch the parent object's records
    – cropredy
    Dec 16, 2016 at 19:42
  • @AdrianLarson - Does the mantra "Clicks not code" apply when someone else wrote the code? +1 for the part about knowing what your doing though
    – Eric
    Dec 17, 2016 at 3:07
  • In this case yes, it still makes the solution declarative. A configurable solution that doesn't require the OP to write any code. I'd say it fits.
    – Adrian Larson
    Dec 17, 2016 at 3:09

2 Answers 2

4

Credit for aggregate result trick of aliasing the field to Id goes to sfdcfox

I believe this can all be reduce to:

Trigger Sum on Contact(after insert, after update, after delete, after undelete){
        Map<Id, Account> accountMap = new Map<Id, Account>();

        for (Contact c : trigger.new == null ? trigger.old : trigger.new)
            if (c.accountId != null)
                accountMap.put(c.AccountId, New Account(Id = c.AccountId));

        Map<Id, AggregateResult> agg = New Map<Id,AggregateResult>(
        [
                Select
                        AccountID Id,
                        Sum(Salary__c) s
                From Contact
                Where AccountId In :accountMap.keySet()
                Group By AccountId
        ]
        );

        for (AggregateResult ar : agg) {
            accountMap.get(
                    (Id) ar.get('AccountId')
            ).ContactsSalarySum__c = (Decimal) ar.get('s');
        }

        update accountMap.values();

}

Essentially you:

  1. Get all account IDs in the trigger
  2. Aggregate the sun of the contact fields into a map of Id to Account
  3. Populate the Values in the Map
  4. update the account records

No need to query account records since you will be summing all contacts for a given account every time the contact trigger fires. Keep in mind that this will not be ideal if you have a model with thousands of contacts related to a single account. If you do you will need to exclude those accounts from this (or any really) code that performs these types of operations

1
  • 2
    +1 for the trigger.new == null ? trigger.old : trigger.new expression; never thought of that
    – cropredy
    Dec 17, 2016 at 4:53
0
  1. I think your for loop to populate sumMap doesn't work when you update 2 (or more) contacts related to the same account, bacause the first put and accountId (key) and a con.salary__c (value), the second's one put the same accountId and an other con.salary__c so the con.salary__c of the first Contact might be lost. You need to do something like

Decimal tmp = sumMap.get(con.accountId); sumMap.put(con.accountId, tmp!==null? con.salary : tmp + con.salary)

BUT SUMMAP IS NEVER REALLY USED.

  1. You have not TWO FOR LOOP but THREE in Update is too much

3.With a different implementation of the Insert Trigger and Delete Trigger you can do a lighter Query (Without nested query).

4.Best practise: Use all the Trigger type (befor insert, after insert , before....) and proper if to handle it. Implement Trigger logiC in a ContactHandler Class.

TRIGGER:

Trigger ContactTrigger on Contact(before insert, after insert, before update, after update, before update, after delete, before undelete, after undelete){
    if(Trigger.isAfter && Trigger.isInsert){
        Map<Id,Decimal> accountSalarySum = new  Map<Id,Decimal>();
        for(Contact con : Trigger.new){
            if(accountSalarySum.get(con.AccountId)!=null){
                accountSalarySum.put(con.AccountId,accountSalarySum.get(con.AccountId) +con.Salary__c)
            } else {
                accountSalarySum.put(con.AccountId,con.Salary__c)
            }
        }
        Set<Id> ids = accountSalarySum.keySet();
        accs = [SELECT Id,ContactsSalarySum__c FROM Account WHERE Id=:ids];
        for(Account acc : accs){
            acc.ContactsSalarySum__c += accountSalarySum(acc.id);
        }   
        update accs;
    }

    if(Trigger.isAfter && Trigger.isDelete){
        Map<Id,Decimal> accountSalarySum = new  Map<Id,Decimal>();
        for(Contact con : Trigger.old){
            if(accountSalarySum.get(con.AccountId)!=null){
                accountSalarySum.put(con.AccountId,accountSalarySum.get(con.AccountId) -con.Salary__c)
            } else {
                accountSalarySum.put(con.AccountId,0-con.Salary__c)
            }
        }
        Set<Id> ids = accountSalarySum.keySet();
        accs = [SELECT Id,ContactsSalarySum__c FROM Account WHERE Id=:ids];
        for(Account acc : accs){
            acc.ContactsSalarySum__c += accountSalarySum(acc.id);
        }
        update accs;
    }

    if(Trigger.isAfter && Trigger.isUpdate){
        Set<Id> ids = new Set<Id>();
        for(Contact con : Trigger.old){
            set.add(con.AccountId);
        }
        Account accs = [SELECT id, ContactsSalarySum__c, (SELECT id, Salary__c FROM Contacts) FROM Account WHERE Id In =: ids]
        for(Account acc : accs){
            Decimal sum = 0;
            for(Contact con : acc.contacts){
                sum = sum + con.salary__c;
            }
            acc.ContactsSalarySum__c = sum;
        }
        update accs;
    } 
}

Enjoy :)

You must log in to answer this question.

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