function readOnly(count){ }
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
+ Start a Discussion
Durvesh Shah 8Durvesh Shah 8 

Avoid Soql query in For loop for grouping related fields

User-added image
 I have grouped my account with respective account owner but i have queried in my for loop which is not a a best practice. Can Someone give me optimized version of my code. It will be of great help. Below is my Apex Class.
 
public with sharing class MapofAccountsExt{

 public List<Account> Accountlist{get;set;}
 public Map<String,List<Account>> AccountMap{get;set;}
 public Map<String,List<User>> UserMap{get;set;}
 
 public MapofAccountsExt(ApexPages.StandardController controller) 
    {
        thismethod();
    }

 public PageReference thismethod()
    {
      Accountlist = new List<Account>();
      Accountlist = [select id,AccountNumber,name,owner.name,email__c,Active__c from Account];
        Set<id> setofids = new Set<id>();
        for(Account thisAccount : Accountlist)
        {
            setofids.add(thisAccount.ownerid);
            system.debug('the whole set of id '+setofids);
        }
        list<Account> newAccountList = new List<Account>();
        
        newAccountList= [Select id,AccountNumber,name,ownerid,owner.name,email__c,Active__c from Account where ownerid in:setofids];
        system.debug('this expense id for the list '+newAccountList);
        
        
        AccountMap= new Map<String,List<Account>>();
        List<Account> accList = new List<Account>();
        for(Account thisAccountid: newAccountList)
        {
                accList = [select id,AccountNumber,name,ownerid,owner.name,email__c,Active__c from Account where ownerid=:thisAccountid.ownerid and (id = '0016F00002PEDkg' or id ='0016F00002PEECU' or id='0016F00002PF4Bt' or id = '0016F00002PF5UK')];
                AccountMap.put(thisAccountid.owner.name,accList);
                system.debug('this Account id '+AccountMap);

        }
        List<User> Userlist = new List<User>();
        Userlist = [select id,name,email from User];
        UserMap = new Map<String,List<User>>();
        List<User> Userlist1 = new List<User>();
        for(User thisUser1 : Userlist)
        {
            Userlist1 = [select id,name,email from User where email=:thisuser1.email limit 1];
            UserMap.put(thisuser1.name,Userlist1);
        }
        
        
        return null;       
    }
}

 
Best Answer chosen by Durvesh Shah 8
Rohit Kumar SainiRohit Kumar Saini
Hi Durvesh,

I am not sure what are hard-coded Account Ids in your code but below should work:
public with sharing class MapofAccountsExt{
    public Map<String,List<Account>> AccountMap{get;set;}//map of user id to list of accounts
    public Map<String,User> UserMap{get;set;}//map of User id to user
    public List<string> userIds{get;set;}   
    
    public MapofAccountsExt(ApexPages.StandardController controller) 
    {	
        thismethod();
    }
    
    private void thismethod()
    {
        //this will query all accounts; assuming you don't have more than 50k accounts, we don't have to query again
        AccountMap=new  Map<String,List<Account>>();
        for(Account acc: [select id,AccountNumber,name, ownerid, owner.name from Account limit 50000]){
            if(AccountMap.get(acc.ownerid)!=null){
                List<Account> existingAccountsForThisUser=	AccountMap.get(acc.ownerid);
                existingAccountsForThisUser.add(acc);
                AccountMap.put(acc.ownerId,existingAccountsForThisUser);
            }else{
                AccountMap.put(acc.ownerId, new List<Account>{acc});
            }
        } 
        //We created map for Account ownerId instead of Account owner name as name can be same for two users which will create issues and show all accounts of those two different users under one user name
        
        UserMap = new Map<string, User>([select id,name, email from User where id in :AccountMap.keyset() ]);
        userIds=new List<string>();
        userIds.addAll(UserMap.keySet());
    }
}

I am also adding how VF page should extract these values..You can use any other way than the  HTML table I used.
<apex:page extensions="MapofAccountsExt" standardController="Account">
    <apex:repeat value="{!userIds}" var="u">
        {!UserMap[u].Name} &nbsp; {!UserMap[u].Email}
        <table>
            <apex:repeat value="{!AccountMap[u]}" var="acc">
                <tr>
                    <td>{!acc.Id}</td>
                    <td>{!acc.Name}</td>
                </tr>
            </apex:repeat>
        </table>
    </apex:repeat> 
</apex:page>

Please let me know if you have any questions. Thanks.

All Answers

Rohit Kumar SainiRohit Kumar Saini
Hi Durvesh,

I am not sure what are hard-coded Account Ids in your code but below should work:
public with sharing class MapofAccountsExt{
    public Map<String,List<Account>> AccountMap{get;set;}//map of user id to list of accounts
    public Map<String,User> UserMap{get;set;}//map of User id to user
    public List<string> userIds{get;set;}   
    
    public MapofAccountsExt(ApexPages.StandardController controller) 
    {	
        thismethod();
    }
    
    private void thismethod()
    {
        //this will query all accounts; assuming you don't have more than 50k accounts, we don't have to query again
        AccountMap=new  Map<String,List<Account>>();
        for(Account acc: [select id,AccountNumber,name, ownerid, owner.name from Account limit 50000]){
            if(AccountMap.get(acc.ownerid)!=null){
                List<Account> existingAccountsForThisUser=	AccountMap.get(acc.ownerid);
                existingAccountsForThisUser.add(acc);
                AccountMap.put(acc.ownerId,existingAccountsForThisUser);
            }else{
                AccountMap.put(acc.ownerId, new List<Account>{acc});
            }
        } 
        //We created map for Account ownerId instead of Account owner name as name can be same for two users which will create issues and show all accounts of those two different users under one user name
        
        UserMap = new Map<string, User>([select id,name, email from User where id in :AccountMap.keyset() ]);
        userIds=new List<string>();
        userIds.addAll(UserMap.keySet());
    }
}

I am also adding how VF page should extract these values..You can use any other way than the  HTML table I used.
<apex:page extensions="MapofAccountsExt" standardController="Account">
    <apex:repeat value="{!userIds}" var="u">
        {!UserMap[u].Name} &nbsp; {!UserMap[u].Email}
        <table>
            <apex:repeat value="{!AccountMap[u]}" var="acc">
                <tr>
                    <td>{!acc.Id}</td>
                    <td>{!acc.Name}</td>
                </tr>
            </apex:repeat>
        </table>
    </apex:repeat> 
</apex:page>

Please let me know if you have any questions. Thanks.
This was selected as the best answer
Durvesh Shah 8Durvesh Shah 8
Thanks a Lot Rohit saini 4, It worked.....
Durvesh Shah 8Durvesh Shah 8
Can you please Explain the if else conditions @Rohit saini 4?
Rohit Kumar SainiRohit Kumar Saini
Hi Durvesh,

AccountMap is a map of User ( Account Owner) and list of Accounts for which the user is the owner. We are iterating over all Accounts. Initially, for any account owner, AccountMap will not have any values so the system will run else statement, in which we are simply putting Account ownerId and new list with current Account in AccountMap. Later, if there are other accounts having the same user as owner, we are getting the existing list from Map, and add current account into that list. Its hard to explain but it is doing almost same thing which you were trying to do earlier ( in your code).

Thanks.
 
Durvesh Shah 8Durvesh Shah 8
Thanks a lot Rohit