4 level, nested for-loop. Bulkified?

Question

I have the following code that works for me. My main concern is the nested for-loops I have toward the end of the code. I'm in the sandbox and don't have enough records to bulk test.

I figured I'd ask you experts whether this is a problem or a violation of a "best practice"?

Thanks in advance.

Update: Structure and goal of code

I am trying to create a record using information from 4 separate objects that have various relationship structures, overview below:

1. Location: this is a "parent" object in the lookups/MD's for every other object in this scenario
2. Receipt: where the trigger originates. This contains a LU to a "Location" record.
3. Inventory Lots: This is a child in LU Relationship to "Location". I'm checking to see if one exists for the location specified on the receipt, if not, create one
4. Storage Metric: This is the Child in a MD relationship to "Location"
5. FG_Transaction: This is the record I'm wanting to create. It takes information from the Receipt and has lookups to Location, Receipt, Inventory_Lots and Storage Metric

This class is the helper to my unified trigger on the Receipt object.

public static void createICFG(Map<Id,Id> RecIds){
        List<Inventory_Lots__c> icList = new List<Inventory_Lots__c>();
        List<FG_Transactions__c> fgList = new List<FG_Transactions__c>();
        RecordType rt = [SELECT Id from RecordType WHERE Name = 'Bulk Batch' AND sObjectType = 'Inventory_Lots__c'];
        RecordType rt2 = [SELECT Id from RecordType WHERE Name = 'Receipt' AND sObjectType = 'FG_Transactions__c'];
        Product_Sesaco__c prd = [SELECT Id from Product_Sesaco__c WHERE Name = 'Commercial, Field Run'];
        Set<Id> icIds = new Set<Id>();

        icList = [SELECT Id, Location__c, Closed__c, On_Hold__c from Inventory_Lots__c WHERE Location__c IN :RecIds.Keyset() AND Closed__c != TRUE AND On_Hold__c != TRUE];
        System.debug('---------------------icList-----------------------------'+icList);

        for(Inventory_Lots__c ic : icList){
            icIds.add(ic.Location__c);
        }

        if(icList.isEmpty()){
        List<Locations__c> locList = [SELECT Id, Name,(SELECT Id, Beginning_Date__c FROM Storage_Metrics__r) from Locations__c WHERE Id IN: RecIds.keySet() AND Id NOT IN :icIds];
            for(Locations__c l : locList){  
                    Inventory_Lots__c icNew = new Inventory_Lots__c();
                    icNew.Name = l.Name+':'+' '+system.date.today().year()+'/'+system.date.today().month()+'/'+system.date.today().day();
                    icNew.Location__c = l.id;
                    icNew.RecordTypeId = rt.id;
                    icNew.Products_sesaco__c = prd.id;
                    icList.add(icNew);
        }
        insert icList;
        }

        System.debug('------------------------------------IC List----------------------------------------'+icList);

        List<Receiving__c> recList = [SELECT Id, GRLBS__c, Glyphosate_Used__c, Date_Received__c, Locations__c FROM Receiving__c WHERE Id IN :RecIds.values() AND Locations__c IN :RecIds.keySet()];

        System.debug('-----------------------------------Rec List---------------------------------------'+RecList);

        List<Locations__c> locList = [SELECT Id, Name, (SELECT Id, Beginning_Date__c FROM Storage_Metrics__r) from Locations__c WHERE Id IN: RecIds.keySet() AND Id IN :icIds];
        for(Locations__c l : locList){
        for(Receiving__c r : recList){
            for(Inventory_Lots__c ic : icList){
                for(Storage_Metric__c sm : l.Storage_Metrics__r){
                if(ic.Location__c == r.Locations__c && r.Date_Received__c.toStartOfMonth() == sm.Beginning_Date__c){
                    FG_Transactions__c fgNew = new FG_Transactions__c();
                    fgNew.Receiving_Number__c = r.id;
                    fgNew.FG_Lots__c = ic.id;
                    fgNew.Locations_primary__c = r.Locations__c;
                    fgNew.Trx_Date__c = r.Date_Received__c;
                    fgNew.Trx_Qty_Lbs__c = r.GRLBS__c;
                    fgNew.RecordTypeId = rt2.id;
                    fgNew.HA_Applied__c = r.Glyphosate_Used__c;
                    fgNew.Storage_Metric__c = sm.id;
                    fgList.add(fgNew);
              }
             }
            }
           }
        }
        insert fgList;
    }

}

Answer 1

I would start off by putting the different locations in maps. So something like:

List<Inventory_Lots__c> icList = [SELECT Id, Location__c, Closed__c, On_Hold__c from Inventory_Lots__c WHERE Location__c IN :recIds.Keyset() AND Closed__c != TRUE AND On_Hold__c != TRUE];
Map<Id, List<Inventory_Lots__c> lotByLocation = new Map<Id, List<Inventory_Lots__c>>();
for (Inventory_Lots__c il: icList)
{
  List<Inventory_Lots__c> lots = lotsByLocation.get(il.Location__c):
  if (lots == null)
  {
    lots = new List<Inventory_Lots__c>();
    lotsByLocation.put(il.Location__c, lots);
  }
  lots.add(il);
}

NOTE-You can do a similar thing when doing the insert as well. Do this also for the Storage_Metrics__c object.

After you create maps for for the Storage_Metrics__c object and Inventory_Lots__c object:

List<FG_Transactions__c> fgList = new List<FG_Transactions__c>();
for (Receiving__r r: recList)
{
  List<Inventory_Lots__c> lots = lotsByLocation.get(r.Location__c);
  if (lots == null)
    continue;
  for (Inventory_Lots__c ic: lots)
  {
    if (!storageMetricsByLocation.containsKey(ic.Location__c))
      continue;
    for (Storage_Metric__c sm: storageMetricsByLocation.get(ic.Location__c))
    {
      //You already know location is the same because of the maps so you dont need to check it
      if (r.Date_Received__c.toStartOfMonth() == sm.Beginning_Date__c)
      {
        FG_Transactions__c fgNew = new FG_Transactions__c();
        fgNew.Receiving_Number__c = r.Id;
        fgNew.FG_Lots__c = ic.id;
        fgNew.Locations_primary__c = r.Locations__c;
        fgNew.Trx_Date__c = r.Date_Received__c;
        fgNew.Trx_Qty_Lbs__c = r.GRLBS__c;
        fgNew.RecordTypeId = rt2.id;
        fgNew.HA_Applied__c = r.Glyphosate_Used__c;
        fgNew.Storage_Metric__c = sm.id;
        fgList.add(fgNew);
      }
    }
  }
  insert fgList;
}

This does all of the same work. In addition to that, there are only 3 nested loops instead of 4. It also doesn't loop over the entire sets of data. It only looks at things that share the same Location__c. So you have 500 total Storage_Metrics__c returned in your query, but only say 10 are related to each Location, you are saving 490 iterations of the loop PER parent. That's a pretty substantial difference.

Not Required Read Notes-Most everything else in there is simply my code style. I like to use the 'continue' simply because it doesn't make me indent over super far. You can just use the if statements with the negation of what mine say and put the work in the loops if you prefer, but it doesn't matter either way.