2

Can somebody help me to use MAPs instead of for loops. My below code is having a query on User object. In Users having Related_Team__c -'Operation' there is related_BDE__C, service_segment__c, both are multi-select picklists. Whenever a BDE changes a Opportunity stage to 'Closed Won' an Operation (a custom object) record is made.
Operation User and BDE user are related to service_Segment. e.g if service segment of Opportunity is 'Company Registration' than only Operation user having a Company Registration in there service_segment__c field is entitled to become the owner of that operation record. And also BDE user should be in Operation User BDE list. Now I am picking the service segment and BDE user so that I can find which Operation user is entitled to have its Operation record.

The code is bit more lengthy if any body like to have complete code I will modify it.

map<String, map<String, list<string>>> relativeOPEBDE = new map<String, map<String, list<string>>>();
list<String> relatedBDEs = new List<String>();
list<String> servSegments = new List<String>();
list<User> tempUser = [SELECT firstname, lastname, Related_BDE__c, Related_Team__c,Service_Segment__c, id 
                       FROM User 
                       WHERE isactive = true 
                       AND On_Leave__c = false 
                       ORDER BY Related_Team__c ASC];

for(User tempUsers : tempUser) {
    systemUserInfo.put(tempUsers.firstname.toLowerCase() + ' ' + tempUsers.lastname.toLowerCase(), tempUsers.id);  
    system.debug('Related Team values ---- ' + tempUsers);

    if (tempUsers.Related_BDE__c != null     && 
        tempUsers.Related_BDE__c != ''       && 
        tempUsers.Service_Segment__c != null &&
        tempUsers.Service_Segment__c != ''      ){

            servSegments = tempUsers.Service_Segment__c.split(';'); 
            relatedBDEs = tempUsers.Related_BDE__c.split(';'); 

            system.debug('--related BDE--'+ relatedBDEs);
            if (relatedBDEs.size() > 0 && servSegments.size() > 0) {
                for (String tempString : relatedBDEs) {
                    tempString = tempString.toLowerCase();  
                    system.debug('tempString values are ----' + tempString);
                    system.debug('--Service Segments--'+ servSegments + '--operation SS--' + serviceSegment);

                    for (String tempSegment : servSegments) {
                        //for(Operation__c tempOperation : trigger.new)
                            //tempSegment = tempSegment.toLowerCase();  
                        system.debug('tempSegment values are ----' + tempSegment);
                        if (tempSegment != null
                            && tempUsers.Related_Team__c == 'Operation') {
                            system.debug('--operationTeam--'+ operationTeam);
                            if (tempSegment == 'Company Registration'
                                || tempSegment == 'LLP Registration') { 
                                    /*
                                    if(tempOperation.Service_Segment__c == 'Company Registration'
                                        || tempOperation.Service_Segment__c == 'LLP Registration')
                                    */  
                                system.debug('--coming here in if--'+ tempSegment);
                                if(operationTeam.containsKey(tempSegment)){

                                    relativeOPEBDE.get(tempSegment).get(systemUserInfo.get(tempString)).add(tempUsers.id);
                                    system.debug('--relativeOPEBDE--' + relativeOPEBDE);
                                }else { 
                                    relativeOPEBDE.put(tempSegment, new map<string, list<string>>{systemUserInfo.get(tempString)=> new list<String>{tempUsers.id}});
                                    system.debug('--relativeOPEBDE--' + relativeOPEBDE);
                                }   
                                system.debug('--Company Registration--' + operationTeam);

                            } 
                            else if(tempSegment == 'Compliance Services') {

                                //if(tempOperation.Service_Segment__c == 'Compliance Services')
                                system.debug('--coming her in ist else if--'+ tempSegment );
                                if(operationTeam.containsKey(tempSegment)){
                                    operationTeam.get(tempSegment).add(tempUsers.id);
                                }else { 
                                    operationTeam.put(tempSegment, new List<String> {tempUsers.id});
                                }   
                                system.debug('--Compliance Services--' + operationTeam);
                            } 
                            else if(tempSegment == 'IPR Services'){
                                //if(tempOperation.Service_Segment__c == 'IPR Services')
                                system.debug('--coming her in 2nd else if--'+ tempSegment );
                                if(operationTeam.containsKey(tempSegment)){
                                    operationTeam.get(tempSegment).add(tempUsers.id);
                                }else { 
                                    operationTeam.put(tempSegment, new List<String> {tempUsers.id});
                                }   
                                system.debug('--IPR Services--' + operationTeam);
                            }  
                        } 
                    }
                }   
            }
        }   
    }

Here in above code I have already use 3 for loops and I want to use another loop. But I know it is not a good practice to use so many loops and also we can use MAPs instead of for loops. Please help me to reduce code line.

  • 1
    While it is a very good idea to improve your implementation and ask for help, it would be easier to help you walk through this if you could include an explination of what your code is intended to do, simplify or break down the functionality and comment your code to some extend. – Samuel De Rycke Mar 5 '14 at 9:48
  • I tried to add more comments. Samuel please go through it and let me know if you required more code or comments – Rohit Mar 5 '14 at 11:45
  • Hi amatorVitae, is it write indenting to right code as you edited if (relatedBDEs.size() > 0 && servSegments.size() > 0) {. Or Should we choose different line indentation in condition statements for clear reading/understanding. – Rohit Mar 6 '14 at 7:47
3

This is something of a guess as the code is not too easy to follow so please use your own judgement on its relevance to your problem.

Your code looks trigger-related, and with triggers it is important that they do as little work as possible. So a trigger would usually start with this sort of loop (rather than have it in the middle as in your code):

for(Operation__c tempOperation : trigger.new)

and just gather the unique values (of e.g. Service_Segment__c) into a map or set.

There would then follow (if the map or set is not empty) the query of the User object using a where clause that is selective as possible and so adds something like this to what you already have:

Service_Segment__c includes :segments
and/or
Related_BDE__c includes :bdes 

And it is inside this second loop that you would include your logic, adding any resulting values into further maps or sets.

If any updates or inserts are needed, these would then be done in a third step.

So overall execution time is proportional to the number of objects rather than growing exponentially (as typically happens where loops are nested within loops).

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.