3

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;
        }
    }
}
3
  • 1
    Are you querying the objects from the database? If so, could you not use an ORDER BY clause to ensure they are pre-sorted?
    – Phil W
    Jun 23 '20 at 13:22
  • that's a good idea however in this case those WOs are not in database yet and is being passed to the above method.
    – SfdcBat
    Jun 23 '20 at 13:26
  • 1
    Fair enough. Worth updating the question to clarify that point...
    – Phil W
    Jun 23 '20 at 13:39
3
  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.

4
  • great idea. However what if I say the uniqueKey is like 005q00000069k2d12/31/2020000002, basically uniqueKey is => a sfdc id + date + Work_Order_Sequence__c.leftPad(10, '0'). Would this approach still work?
    – SfdcBat
    Jun 23 '20 at 14:15
  • @SfdcBat sure just be sure it's truly unique and keep in mind that it will alpha sort unless you write your own sort alg by implementing Comparable Jun 23 '20 at 16:59
  • Yes the key will be unique for sure - 0Hnq00000000DtYCAU2020-06-02 00:00:00 1 . I see there is no need to use leftPad(10, '0') if the key is like sfdc id(0Hnq00000000DtYCAU) + date(2020-06-02 00:00:00)+ Work_Order_Sequence__c(1). What do you think?
    – SfdcBat
    Jun 23 '20 at 17:36
  • 1
    leftPad({maxdigits}, '0') padding is used to sort the Work_Order_Sequence__c properly if you have values >=10 (i.e. values 1, 2, 10, 11, 101 would sort as 1, 10, 101, 11, 2 without padding zeros. Jun 23 '20 at 20:51
4

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.

4
  • thanks I do agree with your thought. Actually, I am dealing with thousands of records in the project and my idea is to reduce as much for loop as possible(if really possible). The above code is just part of a much more functionality. It just adds up.
    – SfdcBat
    Jun 23 '20 at 13:47
  • 1
    @SfdcBat the fifth link actually includes a useful trick that's better for performance (at the cost of some legibility) you might like.
    – sfdcfox
    Jun 23 '20 at 13:49
  • Nice idea. COuld you let me know about your idea on - "sorting before putting them in the map (this would mean only one sort() call ". Can you show me how?
    – SfdcBat
    Jun 23 '20 at 13:53
  • 1
    @SfdcBat Gauge Boson beat me to it, actually. Just sort the records before putting them into the maps; they'll still be sorted when you're done. That said, the performance difference is probably close to zero because sorting is a linear-growth algorithm (in other words, it takes about 100 times longer to sort 100 items than to sort 1 item). The only change is that calling apex methods have some CPU cost, so calling the method once instead of 100 times saves 99 method calls and the corresponding overhead.
    – sfdcfox
    Jun 23 '20 at 13:58

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.