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

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
    Jan 12 '17 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
    Jan 10 '17 at 21:34

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.