0

I am working on something that requires insert, update and delete actions on Account object. I have written the below code but would like to know if there's a better way to do this by using maps instead of using two SOQL for Deleting and updating.

try{      
        //SavePoint and Rollback logic is added
        sp = Database.setSavepoint();

        string normalSecurityCode = Global_Constants.NORMAL_SECURITY_CODE;

        List<Confidential_Client__c> insertCCs = new List<Confidential_Client__c>();
        List<Confidential_Client__c> updateCCs = new List<Confidential_Client__c>();
        List<Confidential_Client__c> deleteCCs = new List<Confidential_Client__c>(); 


        List<Account> newConfidentialClients = new List<Account>();
        List<Account> updatedConfidentialClients = new List<Account>();
        List<Account> notConfidentialClients = new List<Account>(); 

        for(Account tempAccount : newAccounts)
        {

            if(oldMapAccounts != NULL && oldMapAccounts.containsKey((ID)tempAccount.get('Id')))
            {                                     
                if( oldMapAccounts.get((ID)tempAccount.get('Id')).get('Client_Security_Code__c') != tempAccount.get('Client_Security_Code__c') 
                && tempAccount.get('Client_Security_Code__c') == normalSecurityCode){

                    notConfidentialClients.add(tempAccount); 

                }
                else if(oldMapAccounts.get((ID)tempAccount.get('Id')).get('Client_Security_Code__c') != tempAccount.get('Client_Security_Code__c') 
                && tempAccount.get('Client_Security_Code__c') != normalSecurityCode){
                    newConfidentialClients.add(tempAccount);
                }
                else if(oldMapAccounts.get((ID)tempAccount.get('Id')).get('Client_Security_Code__c') == tempAccount.get('Client_Security_Code__c') 
                && tempAccount.get('Client_Security_Code__c') != normalSecurityCode)
                {
                    Account oldAccount = oldMapAccounts.get((ID)tempAccount.get('Id'));

                    if(oldAccount.Name != tempAccount.Name || oldAccount.Email__c != tempAccount.Email__c 
                       ||oldAccount.Mobile_Phone__c != tempAccount.Mobile_Phone__c || oldAccount.Home_Phone__c != tempAccount.Home_Phone__c ||
                       oldAccount.Other_Phone__c != tempAccount.Other_Phone__c || oldAccount.Work_Phone__c != tempAccount.Work_Phone__c)
                       {

                           updatedConfidentialClients.add(tempAccount);
                       }


                }
            }
            else if(oldMapAccounts == NULL && tempAccount.get('Client_Security_Code__c') != normalSecurityCode){
                newConfidentialClients.add(tempAccount);
            }
        }

        if (!notConfidentialClients.isEmpty())
        {

            deleteCCs = [SELECT id,name,Client__c FROM Confidential_Client__c WHERE Client__c in :notConfidentialClients];

            if (!deleteCCs.isEmpty()){
                delete deleteCCs;  
            }
        }

        if (!newConfidentialClients.isEmpty())
        {
            for (Account tempAccount:newConfidentialClients) 
            {
                insertCCs.add(
                    new Confidential_Client__c(
                        Name = tempAccount.Name,
                        Email_Searchable__c = tempAccount.Email__c,
                        Mobile_Phone_Searchable__c = tempAccount.Mobile_Phone__c,
                        Home_Phone_Searchable__c=tempAccount.Home_Phone__c, //Added as part of CRMT-77787
                        Client__c = tempAccount.Id,
                        Other_Phone_Searchable__c=tempAccount.Other_Phone__c,
                        Work_Phone_Searchable__c=tempAccount.Work_Phone__c
                    )
                );
            }

            if (!insertCCs.isEmpty()){
                insert insertCCs;  
            }
        }
        if(!updatedConfidentialClients.isEmpty()){

            for (Confidential_Client__c cc: [SELECT Id,Name,Client__c FROM Confidential_Client__c WHERE Client__c in :updatedConfidentialClients]) 
            {
                Account a = newAccountMap.get(cc.Client__c);
                cc.Name = a.Name;
                cc.Email_Searchable__c = a.Email__c;
                cc.Mobile_Phone_Searchable__c = a.Mobile_Phone__c;
                cc.Home_Phone_Searchable__c = a.Home_Phone__c;            
                cc.Other_Phone_Searchable__c = a.Other_Phone__c;
                cc.Work_Phone_Searchable__c = a.Work_Phone__c;
                updateCCs.add(cc);
            }
            if (!updateCCs.isEmpty()){
                update updateCCs;  
            }    
        }

    }

Thanks in advance!

1

Different Optimizations

The better way to optimize this code is to remove all of the unnecessary dynamic access and spurious empty checks.

There's no need to use this kind of dynamic access when you have statically typed Account objects:

        if(oldMapAccounts != NULL && oldMapAccounts.containsKey((ID)tempAccount.get('Id')))

This should just be

        if (oldMapAccounts != null && oldMapAccounts.containsKey(tempAccount.Id))

I don't see anywhere you're using get() where it's needed - just use static access.

This code

    if (!notConfidentialClients.isEmpty())
    {

        deleteCCs = [SELECT id,name,Client__c FROM Confidential_Client__c WHERE Client__c in :notConfidentialClients];

        if (!deleteCCs.isEmpty()){
            delete deleteCCs;  
        }
    }

reduces to

delete [SELECT id,name,Client__c FROM Confidential_Client__c WHERE Client__c in :notConfidentialClients];

You don't need any of the isEmpty() checks. Deleting an empty list does nothing. Iterating over an empty list does nothing.

The Direct Question

... if there's a better way to do this by using maps instead of using two SOQL for Deleting and updating.

You could, in theory, rebuild your code to run only one query on Confidential_Client__c to locate those implicated in either the delete or update steps, and then sort them into separate lists via Apex. I wouldn't, personally, unless I'd done profiling that showed a specific performance or limits impact that would be ameliorated by that change.

Using two SOQL queries seems more natural and understandable than such a solution to me, and the queries are likely (knowing nothing about your org) to be performant since they filter on a reference field.

  • That's a great explanation. Thanks! – SunnyG Oct 31 '19 at 22:49

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.