asked on

Salesforce Apex Trigger best practice

Hello,

I was hoping to get some feedback on an Apex trigger I have written on the Salesforce platform. I have two custom objects (SalesOrder__c and Sales_Order_Line__c) that is a master/detail relationship.

On an edit of the line, I'm iterating over all the lines, running a check to see if the lines have been fully fulfilled and if so, then setting a fully fulfilled Boolean field on the master record to true. The check on the lines is looking at a custom formula field and the record type of the lines.

I'm interested in any feedback on the code I have written (see below) regarding best practices on bulkification, code logic, error handling, appropriate DML usage and governor limits.

For example, is it better to use standard DML operation or the database DML methods and use partial success? Should try/catch be used in triggers? I've read differing opinions on using error handling in triggers. Would a different loop be more efficient (do/while)? Should limit checks be added into the code? Any feedback on improvements and best practices would be welcome.

Please note, I'm using a trigger framework that uses a handler interface. The trigger calls the framework class, passing in the event to fire the trigger and the handler class to dispatch. The code below is the handler class that runs the logic on trigger after update event.

global class SalesOrderLineFullyFulfilled implements ITriggers.HandlerInterface {

	Sales_Order_Line__c[] newCollection = trigger.new;
	
	global void handle() {
		system.debug('************************** SalesOrderLineFullyFulfilled START');
		setFullyFulfilled(getSalesOrderLineMap(getSalesOrderIds()));
		system.debug('************************** SalesOrderLineFullyFulfilled END');
	} 
	
	private Set<Id> getSalesOrderIds() {
		Set<Id> salesOrderIds = new Set<Id>();
		for(Sales_Order_Line__c sol : newCollection) {
			salesOrderIds.add(sol.Sales_Order__c);
		}
		return salesOrderIds;
	}  
	
	private Map<SalesOrder__c, List<Sales_Order_Line__c>> getSalesOrderLineMap(Set<Id> salesOrderIds) {
		Map<SalesOrder__c, List<Sales_Order_Line__c>> mapOrderToOrderLines = new Map<SalesOrder__c, List<Sales_Order_Line__c>>();
		if(salesOrderIds.size() > 0) {
			List<Sales_Order_Line__c> listSalesOrderLines = new List<Sales_Order_Line__c>();
			for(SalesOrder__c so: [select Id, Fully_Fulfilled__c, (select Id, Sales_Order__c, Qty_Ordered_vs_Qty_Packed__c, RecordType.Name from Sales_Order_Lines__r) from SalesOrder__c where Id in: salesOrderIds]) {
				for(Sales_Order_Line__c sol : so.Sales_Order_Lines__r) {
					listSalesOrderLines = mapOrderToOrderLines.get(so);
					if(listSalesOrderLines == null) {
						listSalesOrderLines = new List<Sales_Order_Line__c>();
						mapOrderToOrderLines.put(so, listSalesOrderLines);
					}
					listSalesOrderLines.add(sol);
				}		
			}
		}
		return mapOrderToOrderLines;
	} 
	
	private void setFullyFulfilled(Map<SalesOrder__c, List<Sales_Order_Line__c>> mapOrderToLines) {
		if(mapOrderToLines.size() > 0) {
			Map<SalesOrder__c, SalesOrder__c> recordsToUpdate = new Map<SalesOrder__c, SalesOrder__c>();
			Set<SalesOrder__c> salesOrders = mapOrderToLines.keySet();
			for(SalesOrder__c so : salesOrders) {
				List<Sales_Order_Line__c> listSalesOrderLines = mapOrderToLines.get(so);
				Boolean isFullyFulfilled = true;
				for(Sales_Order_Line__c line : listSalesOrderLines) {
					if(line.RecordType.Name == 'Blank Product' && line.Qty_Ordered_vs_Qty_Packed__c < 0) {
						isFullyFulfilled = false;
						break;
					}
				}
				so.Fully_Fulfilled__c = isFullyFulfilled;
				recordsToUpdate.put(so,so);
			}
			if(recordsToUpdate.size() > 0) {
				update recordsToUpdate.values();
			}
		}
	}
}

Open in new window

Kevin Cross

It looks good at initial glance to me. In a situation I just dealt with, we went with a Queued Job since we did not need the action to complete synchronously with the inserts|updates of the trigger. To handle this, I just put a nested Queueable inside my original class.

public class AsyncSalesOrderLineFullyFulfilled implements Queueable {
        private Set<Id> salesOrderIds;
        //---- construct new instance
        public AsyncSalesOrderLineFullyFulfilled (Set<Id> salesOrderIds) {
            this.salesOrderIds = salesOrderIds;
        }
        //---- perform the magic
        public void execute(QueueableContext context) {
            // do what you need here
            // in my code, I used static calls e.g.
            SalesOrderLineFullyFulfilled.setFullyFulfilled(getSalesOrderLineMap(salesOrderIds));
        }
    }

Open in new window

EDIT: if you go with this approach, instead of calling setFullyFulfilled inside of handle(), you would enqueue job.

 ID jobID;
jobID = System.enqueueJob(new AsyncSalesOrderLineFullyFulfilled(getSalesOrderIds()));
System.debug(String.format('Started Async Job {0}.', new String[]{jobID}));

Open in new window

/EDIT

In my implementation, I went with Database class because I didn't want the whole batch to fail because one update went bad.

Database.SaveResult[] rs = Database.update( recordsToUpdate.values(), false );

Open in new window

I hope that helps.

P.S. please forgive any code type-o's. Copy and paste from my class in Salesforce but then hand typed here, so above is not compiled syntax. Just example.

ASKER CERTIFIED SOLUTION

Kevin Cross

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

jkurant

I think there is a much easier solution. I assume that your formula field Qty_Ordered_vs_Qty_Packed__c is just the Qty_Ordered__c minus the Qty_Packed__c.

So, I think you could add two new rollup fields on your master record, Sales_Order__c, one that does a sum of all Qty_Ordered__c and one that does a sum of all Qty_Packed__c on Sales_Order_Lines__c. If the two numbers are the same, then you have entirely fulfilled all of the order lines in the order.

You can then make a formula checkbox field doing a comparison of the two rollup fields and have it checked when they are equal and not checked when they are not equal.