Why my Map having same values for different Key?

Question

I'm having Requirement to find out get Unique Domain emails of child Contact and update Parent Account field on Domain_Count__c with their related child contact unique Emails.

But while debugging It's showing same value for different Account Ids. Can Someone help me out with this....

I've written below for requirement:

public class Domains{
    public static void getuniqueDomain(List<Contact> conlist){
        Set<String> domainNamesSet = new Set<String>();
        Map<Id,Set<String>> AccIdAndEmailSetMap = new Map<Id,Set<String>>();
        Set<Id> ParentAccountIdset = new Set<Id>();
        List<Account> AccountsTobeUpdate = new List<Account>();
        for(Contact con : conlist){
            ParentAccountIdset.add(con.AccountId);
        }
        for(Account a : [SELECT Id,Domain_count__c,(SELECT Id,Email FROM Contacts) FROM Account WHERE Id IN: ParentAccountIdset]){
            domainNamesSet.clear();
            for(Contact c : a.Contacts){
                if(c.Email!=null){
                    domainNamesSet.add(c.Email.Split('@').get(1)); 
                }
            }
            AccIdAndEmailSetMap.put(a.Id,domainNamesSet);
        }
        for (Id key: AccIdAndEmailSetMap.keySet()) {
            System.debug(LoggingLevel.DEBUG, 'key: ' + key + ' --> value: ' + AccIdAndEmailSetMap.get(key));
            }
        for(Id ids : AccIdAndEmailSetMap.keySet()){
            Account a = new Account(Id=ids , Domain_count__c = AccIdAndEmailSetMap.get(ids).size());
            AccountsTobeUpdate.add(a);
        }
        update AccountsTobeUpdate;
    }
}  

I'm calling this class in my trigger.

Answer 1

As Phil said objects are stored by reference and you have just one domain set in that class since to create a new reference you have to create a new instance. In the SOQL for loop you're relating that one set to every Account's id, that's why when you're printing the map you see always exactly the same domains, no matter what the key is.

Moreover instead of having the if(c.Email!=null) you can move that condition in the subquery: (SELECT Id,Email FROM Contacts WHERE Email != null). This way you'll retrieve only the valid contacts and you can remove the if.

Lastly if you store the current value of Domain_count__c for each Account, you can use such information to update them only if that value's changed.

public class Domains{
    public static void getUniqueDomain(List<Contact> conlist) {
        Set<Id> parentAccountIdSet = new Set<Id>();
        for (Contact con : conlist) {
            parentAccountIdSet.add(con.AccountId);
        }

        Map<Id, Set<String>> mapAccountIdToEmailDomains = new Map<Id, Set<String>>();
        Map<Id, Decimal> mapAccountIdToDomainsCount = new Map<Id, Decimal>();
        for(Account a : [SELECT Id,Domain_count__c,( SELECT Id,Email FROM Contacts WHERE Email != null) FROM Account WHERE Id IN: ParentAccountIdset]){
            mapAccountIdToDomainsCount.put(a.Id, a.Domain_count__c);
            Set<String> domainNamesSet = new Set<String>();
            for (Contact c : a.Contacts) {
                domainNamesSet.add(c.Email.Split('@').get(1)); 
            }
            mapAccountIdToEmailDomains.put(a.Id, domainNamesSet);
        }
        for (Id key: mapAccountIdToEmailDomains.keySet()) {
            System.debug(LoggingLevel.DEBUG, 'key: ' + key + ' --> value: ' + mapAccountIdToEmailDomains.get(key));
        }

        List<Account> accountsToUpdate = new List<Account>();
        for (Id accountId : mapAccountIdToEmailDomains.keySet()) {
            Decimal domainsCount = mapAccountIdToEmailDomains.get(accountId).size();
            if (domainsCount != mapAccountIdToDomainsCount.get(accountId)) {
                Account a = new Account(Id=accountId , Domain_count__c = domainsCount);
                accountsToUpdate.add(a);
            }
        }
        update accountsToUpdate;
    }
}

By the way, variable names and method names should be written in lowerCamelCase.