can we avoid 2nd for loop

Question

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;
}

Answer 1

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

Answer 2

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.

2. 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 :)