1

I have a visualforce page that uses a junction object to display child assets. The page functions as expected, however when more than 100 records are returned, the VF Page breaks with the dreaded error: "System.LimitException: Too Many SQOL Queries: 101."

How can I refactor the Controller so that the following code is properly bulkified?

public class ViewAssetsController {

    ManageAssetController am = new ManageAssetController();
    Id accid = am.accountId;
    //Wrapper Class
    public class AssetWrap{   

        public List<Child_Assets__c> Child {get; set;}
        public Asset Parent {get;set;}
        public AssetWrap(Asset relParent, List<Child_Assets__c> relChildList){
            Child = relChildList;
            Parent = relParent;
        }
    }

    public Account currentAcc {get; set;}
    public ViewAssetsController(ApexPages.StandardController controller) {
        currentAcc = (Account)controller.getRecord();
    }
    public List<AssetWrap> AssetTree;

    public List<AssetWrap> GetNodes(){
        AssetTree = new List<AssetWrap>();
        List<Asset> relParentList = [SELECT Id, Name, AccountId, Asset_Number__c, Children__c, Type__c, Status, 
                                     Quantity, Start_Date__c, End_Date__c
                                     FROM Asset WHERE AccountId =: currentAcc.Id AND Children__c > 0];

        for (Integer i = 0; i < relParentList.size(); i++){
            List<Child_Assets__c> relChildList = [SELECT Parent_Asset__c, Child_Asset__c, Child_Asset__r.Name,
                                                       Child_Asset__r.Type__c, Child_Asset__r.Status, Child_Asset__r.Quantity, 
                                                       Child_Asset__r.Start_Date__c, Child_Asset__r.End_Date__c
                                                       FROM Child_Assets__c WHERE Parent_Asset__c =: relParentList[i].Id];
            AssetTree.add(new AssetWrap(relParentList[i], relChildList));
        }      
        return AssetTree;
    }
    public PageReference viewAssets(){
        PageReference ret = new PageReference('/apex/ViewAssets?id='+accId);
        ret.setRedirect(true);
        return ret;
    }
}

2 Answers 2

2

You can simply move the query before your loop. Use this common pattern to group the child records by their parent Id:

Map<Id, List<Child_Asset__c>> assetIdToChildren = new Map<Id, List<Child_Asset__c>>();
for (Child_Asset__c child : [
    SELECT ... FROM Child_Asset__c
    WHERE Parent_Asset__c IN :relParentList
]){
    if (!assetIdToChildren.containsKey(child.Parent_Asset__c))
        assetIdToChildren.put(child.Parent_Asset__c, new List<Child_Asset__c>());
    assetIdToChildren.get(child.Parent_Asset__c).add(child);
}

Once you have that cached data in place, you're good to go:

for (Asset parent : relParentList)
{
    List<Child_Asset__c> children = assetIdToChildren.get(parent.Id);
    // now you don't need to query in a loop!

    if (children == null) children = new List<Child_Asset__c>();
    // unlike a query, this mapping can return a null List<Child_Asset__c>

    AssetTree.add(new AssetWrap(parent, assetIdToChildren.get(parent.Id));
}
1
  • This solution worked. The Mapping removed the query from inside the for loop. Thank you!
    – user8556
    Commented Jan 12, 2017 at 19:06
2

I think your realized that your problem is the SQL in the for loop. It's best practice to avoid this absolutely. A good way to go is to use more Map. I think you will find a few other post about this on SFSE.

Here is how it would look like (not sure if it's compiling but should be closed to that) :

AssetTree = new List<AssetWrap>();
    Map<Id, Asset> relParentList = new Map  Map<Id, Asset> ([SELECT Id, Name, AccountId, Asset_Number__c, Children__c, Type__c, Status, Quantity, Start_Date__c, End_Date__c FROM Asset WHERE AccountId =: currentAcc.Id AND Children__c > 0]);

    List<Child_Assets__c> relChildList = [SELECT Parent_Asset__c, Child_Asset__c, Child_Asset__r.Name,
                                               Child_Asset__r.Type__c, Child_Asset__r.Status, Child_Asset__r.Quantity, 
                                               Child_Asset__r.Start_Date__c, Child_Asset__r.End_Date__c
                                               FROM Child_Assets__c WHERE Parent_Asset__c IN  :relParentList.keySet()
                                               ORDER BY Parent_Asset__c];
    for (Child_Assets__c child : relChildList){
        AssetTree.add(new AssetWrap(relParentList.get(child.Parent_Asset__c), child));    
    }
    return AssetTree;

A few good articles for you to checkout :

Apex Governor Limits

Apex Code Best Practices

1
  • I think you may have misunderstood how the AssetWrap class works, since its constructor accepts a single parent and many children.
    – Adrian Larson
    Commented Jan 10, 2017 at 21:34

You must log in to answer this question.

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