Efficiently sorting a list of Work Order records by a Sequence

Question

I have Work_Order_Sequence__c and UniqueKey__c field on Work Order object. In my method I wish to create a Map which has a Key => UniqueKey__c and Value=> List<WorkOrder> (sorted). Basically grouping all the sorted work orders by UniqueKey__c. Unfortunately I do not have option to do ORDER BY as these WO are not in database.

Example:
--------
#.          Work_Order_Sequence__c         UniqueKey__c

WO1                 3                          abc
WO2                 1                          abc
WO3                 2                          abc
WO4                 1                          xyz
WO5                 3                          xyz
WO6                 2                          xyz

The method I am creating takes the unsorted List<WorkOrder> as parameter.

I tried the below approach to sort the WorkOrders and group them by UniqueKey__c. However I see there are two for loops involved here. Is there a more efficient way or more simplified way of achieving this?

What I tried:

public with sharing class SortClass {
    public Map<String, List<WorkOrder>> groupWorkOrders(List<WorkOrder> woList) {
        Map<String, List<WorkOrderWrapper>> mapOfWrapByUnqKey = new Map<String, List<WorkOrderWrapper>>();
        Map<String, List<WorkOrder>> mapOfWOByUnqKey = new Map<String, List<WorkOrder>>();
        for (WorkOrder w : woList) {
            if (mapOfWOByUnqKey.containskey(w.uniqueKey__c)) {
                mapOfWOByUnqKey.get(w.uniqueKey__c).add(w);
                mapOfWrapByUnqKey.get(w.uniqueKey__c).add(new WorkOrderWrapper(w));
            } else {
                mapOfWOByUnqKey.put(w.uniqueKey__c, new List<WorkOrder>{ w });
                mapOfWrapByUnqKey.put(w.uniqueKey__c, new List<WorkOrderWrapper>{ new WorkOrderWrapper(w) });
            }
        }

        for (String s : mapOfWrapByUnqKey.keyset()) {
            mapOfWrapByUnqKey.get(s).sort();
            List<WorkOrder> wolst = new List<WorkOrder>();
            for (WorkOrderWrapper wr : mapOfWrapByUnqKey.get(s)) {
                wolst.add(wr.wo);
            }
            mapOfWOByUnqKey.put(s, wolst);
        }

        return mapOfWOByUnqKey;
    }

    public class WorkOrderWrapper implements Comparable {
        public WorkOrder wo;

        // Constructor
        public WorkOrderWrapper(WorkOrder w) {
            wo = w;
        }

        // Compare workOrders based on the WorkOrder seq.
        public Integer compareTo(Object compareTo) {
            // Cast argument to WorkOrderWrapper
            WorkOrderWrapper compareTowo = (WorkOrderWrapper) compareTo;

            // The return value of 0 indicates that both elements are equal.
            Integer returnValue = 0;
            if (wo.Work_Order_Sequence__c > compareTowo.wo.Work_Order_Sequence__c) {
                // Set return value to a positive value.
                returnValue = 1;
            } else if (wo.Work_Order_Sequence__c < compareTowo.wo.Work_Order_Sequence__c) {
                // Set return value to a negative value.
                returnValue = -1;
            }

            return returnValue;
        }
    }
}

Answer 1

1. Consider mapping all your WorkOrders/Wrappers by a combination key (i.e UniqueKey__c + '.'+ Work_Order_Sequence__c.leftPad(10, '0') ).

2. sort your combo key list comboKey = new List(mapWO.keyset()); combokey.sort();

3. iterate through sorted combokey and create a new list of WorkOrders/Wrappers.

    LIST<WO> wo = new LIST<WO>();
    for (string k : comboKey )
    wo.add(mapWO.get(k));
   

should not need the to implement comparable since you are converting to padded number and can just leverage the standard alpha sort.

Answer 2

As I've said ... at ... least ... four times, just because you have a for loop inside another for loop, does not automatically make this a Bad Thing. Feel free to read the answers for more information. There are some minor optimizations that could improve performance, such as not calling the Map.get method more than once per loop, sorting before putting them in the map (this would mean only one sort() call), and pre-initializing the map keys to remove the IF statements, but these hardly make a difference unless you're dealing with lists of thousands and need that extra kick.

Basically, your code is fine, don't worry about it.