0

Kindly guide me, how I can avoid nested For loop. I have already used Map.

if (reqserviceAcct2assetLst.size() == 0 && serviceAcct2product.size() > 0) { 
    for(Id i : serviceAcct2product.Keyset()){
        for(Id rltdid : serviceAcct2product.get(i)){
            boolean requiredPrdFound = false;

            if( replacepdrctsIds.Containskey(rltdid)){
                for(id reqPrdId: replacepdrctsIds.get(rltdid)){
                    if(setOfProductIds.contains(reqPrdId) && rltdpdrct2originPdrctmap.containsKey(reqPrdId)){
                        requiredPrdFound = true;
                    }
                }

                if(!requiredPrdFound){
                    itemDetails itemerror = new itemDetails();
                    errorMessage = 'For '+ serviceAcctMap.get(i).Name + ', ' + 'you have "' + mainpdrctnamemap.get(rltdid) + '", and need';

                    for(id reqProdId: replacepdrctsIds.get(rltdid)){
                        errorMessage += ' "' + rltdpdrctnamemap.get(reqProdId) + '" or';
                    }
                    if(errorMessage.endsWith('or')){
                        errorMessage.removeEndIgnoreCase(' or');
                    }

                    errorMessage += '.';

                    itemerror.Product2Id = rltdid;
                    itemerror.error = errorMessage; 
                    itemerror.serviceAcctId = i;
                    itemList.add(itemerror);
                }
            }
        }
    }
  • Thank you Derek Sir for the formatting. Apologize for missing it. – Suraj Feb 16 at 13:40
  • 1
    No worries, you did a lot better than most new users in that regard. It does take some time to consistently format code, but being consistent really helps others in reading your code. As a quick tip, I usually correct formatting by hand, but you can use the execute anonymous window in the dev console to do it automatically. Copy/paste your code into an execute anonymous window, then press shift + tab. It won't work 100% of the time, but it tends to do a pretty nice job. – Derek F Feb 16 at 13:45
  • Wow. thank you so much. I already learnt new things after joining. Appreciate it Sir. – Suraj Feb 16 at 13:46
  • 2
    It'll be easier to help if you can include a description of what this code actually does, simply because it reduces the time a reader must spend to understand what you're trying to do. I recently wrote an answer to a similar question about nesting for loops. – David Reed Feb 16 at 13:48
  • Thank you @DavidReed. Apologies for not giving the description. There is a custom Object, that holds a relationship. Example:: Product ABC needs Product XYZ. I will have to check all the quote line items and get the Products associated with that and then check this Custom Object to see, if the related product is available. If not, Throw an error. – Suraj Feb 16 at 13:52
1

Nested loops are not inherently evil, and are sometimes required.

Taking an example I saw David Reed use a few days ago, the real problem with nested loops is when you see things like this

for(Opportunity opp :oppsList){  // let's say you have a number, m, of opporunities
    for(Account acc :acctsList){ // and a number, n, of accounts
        // In that case, then this if statement will be evaluated n * m times
        if(acc.Id == opp.AccountId){
            // do things
        }
    }
}

The above is bad, because it checks every account to see if our current opportunity is related to it and we know we can do better. If we have a map of our accounts, we can remove the inner loop entirely

for(Opportunity opp :oppsList){
    Account acc = accountsMap.get(opp.AccountId);
    // do stuff
}

What you're doing here is mostly harmless. If the key of your map isn't included somewhere in the value stored for that key, then there isn't a way to get the key of your map if you iterate over the values.

For things like Map<Id, Account> acctsMap = new Map<Id, Account>([SELECT Id, Name FROM Account]);, the key (the Account Id) is part of the map value.

For something like a Map<Id, List<Product2>> (let's pretend the Id here is that of an Account), you wouldn't be able to get the Account Id given the values (a list of products). Furthermore, to iterate over all of the products in such a map, you will need a nested loop (the outer loop to iterate over the map's keyset, which gets you access to the List<Product2> values, which requires an inner loop to iterate over all of the products in your list).

I'd argue that your situation here is more like the latter. Nested collections require nested loops.

The nested loop here is fine because you aren't wasting any iterations of your loop. Your Id value in the key of your map is being iterated over only once.
You're only iterating over each product id (which I assume is a list) for each map key once.
There is nothing here that can be improved upon (in terms of number of loops, at least).

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.