2

I tried implementing Tony Scott's tidy trigger pattern and failed to understand why there is a separate method for each trigger event apart from bulkBefore() & bulkAfter() methods. Why non-bulk handler methods take single sObject records? Why not do all the processing in bulkBefore() & bulkAfter() methods by calling separate apex classes ? That could be another topic/question.

I switched to lightweight Trigger framework for the interest of time - Details

I'm trying to override the attachment name with parent record name. I have a similar code in before Insert which works. But the before update handler method is not working as expected.

Before Insert code which works fine -

public void BeforeInsert(List<SObject> newRecs) 
{
    set<id> itemids = new set<id>();
    for(attachment att: (List<attachment>)newRecs){
        if(att.parentid.getSobjectType() == Expense_Item__c.sObjectType){
            itemids.add(att.ParentId);
        }
    }

    Map<Id,expense_item__c> itemMap = new Map<Id, expense_item__c>([select name from expense_item__c where id in :itemids]);

    for(attachment att:(List<attachment>)newRecs)
    {
        if((att.parentid.getSobjectType() == Expense_Item__c.sObjectType) && itemMap.containsKey(att.ParentId))
        att.Name= itemmap.get(att.ParentId).name;
    }

}

Before Update event code which's not working.

public void BeforeUpdate(Map<Id, SObject> newRecMap, Map<Id, SObject> oldRecMap) {

    list<attachment> newRecs = newRecMap.values();
    set<id> itemids = new set<id>();
    for(attachment att: (List<attachment>)newRecs){
        if(att.Parentid.getSobjectType() == Expense_Item__c.sObjectType){
            itemids.add(att.ParentId);
        }
    }

    Map<Id,expense_item__c> itemMap = new Map<Id, expense_item__c>([select name from expense_item__c where id in :itemids]);

    for(attachment att: newRecs)
    {
        if((att.parentid.getSobjectType() == Expense_Item__c.sObjectType) && itemMap.containsKey(att.ParentId))
        att.Name= itemmap.get(att.ParentId).name;
    }
  • 2
    Could you put your trigger code as well as well? Before say anything, all code need to find a mistake. Probably, before update statement is missing in trigger, or something else – kurunve Jan 29 '16 at 14:44
  • @kurunve I'm using a single trigger as per the pattern. There's almost no code in my trigger – Jarvis Jan 29 '16 at 17:28
2

I'm going to attempt to answer at least part of your question regarding the Tony Scott's TriggerFactory. Here's some of his code taken from an org I'm working in right now that just includes the bulk handler methods for the base class:

/**
 * private static method to control the execution of the handler
 *
 * Arguments:   ITrigger handler - A Trigger Handler to execute
 */
private static void execute(ITrigger handler)
{
    // Before Trigger
    if (Trigger.isBefore)
    {
        // Call the bulk before to handle any caching of data and enable bulkification
        handler.bulkBefore();

        // Iterate through the records to be deleted passing them to the handler.
        if (Trigger.isDelete)
        {
            for (SObject so : Trigger.old)
            {
                handler.beforeDelete(so);
            }
        }
        // Iterate through the records to be inserted passing them to the handler.
        else if (Trigger.isInsert)
        {
            for (SObject so : Trigger.new)
            {
                handler.beforeInsert(so);
            }
        }
        // Iterate through the records to be updated passing them to the handler.
        else if (Trigger.isUpdate)
        {
            for (SObject so : Trigger.old)
            {
                handler.beforeUpdate(so, Trigger.newMap.get(so.Id));
            }
        }
    }
    else
    {
        // Call the bulk after to handle any caching of data and enable bulkification
        handler.bulkAfter();

        // Iterate through the records deleted passing them to the handler.
        if (Trigger.isDelete)
        {
            for (SObject so : Trigger.old)
            {
                handler.afterDelete(so);
            }
        }
        // Iterate through the records inserted passing them to the handler.
        else if (Trigger.isInsert)
        {
            for (SObject so : Trigger.new)
            {
                handler.afterInsert(so);
            }
        }
        // Iterate through the records updated passing them to the handler.
        else if (Trigger.isUpdate)
        {
            for (SObject so : Trigger.old)
            {
                handler.afterUpdate(so, Trigger.newMap.get(so.Id));
            }
        }
    }

    // Perform any post processing
    handler.andFinally();
}

What I hope you'll notice is that there are only two places in the bulk before and bulk after where the code isn't essentially doing a record by record iteration. That place is at the beginning of each bulk method before any of the if statements begin. Once you move further into the bulk handlers, you begin iterating in for loops. That's how his pattern is intended to work. It's not intended to process any records other than in a record by record basis.

If you want to have methods like yours, since both of them contain queries, what you'd want to do put a section in the andFinally section that either adds that code using if(trigger.isBefore && trigger.isInsert) to which in the first case you pass all of the trigger.new so records to it for processing (pass both the trigger.new and trigger.old records in the 2nd the second example).

From there you can either pass them to another trigger handler class for processing or you can run the query from inside the andFinally method since it's doesn't run inside of a loop.

I'll add that this pattern requires you to think differently than what you've been used to with the way you're taught to write triggers using the standard documentation. It takes some time to adapt to it.

Edit 1 in response to comments

DML is intended only to occur either in the andFinally or in a Helper Class (preferred) that's called from the andFinally method.* SOQL can occur at the beginning of the Trigger Handler Class, in a Helper Class, or in the andFinally method. As I recall, Tony's model shows it occurring in a separate Helper class that runs in a before context to run a query to get Accounts on Opportunity. I believe it's called from the start method or "constructor" section of the Trigger Object Handler.

You may find it helpful to try and think of these initial Trigger Object Handlers that are being spawned by his TriggerFactory as something along the lines of a "Dispatcher" class. Use these Dispatcher Classes to send ALL of your records to another Helper Class that later sorts and processes your records for you according to the logic you want to use. In essence, these Dispatchers only bulk your records for you. If you try to use them to sort records for you, and you have either a lot of different helpers or complex logic, they can very quickly get really messy and incredibly difficult to work with.

It's best to send the real work you want to do to methods that are in other supporting helper classes for each trigger object handler you have in your TriggerFactory classes. In other words, don't expect to only have one class that does all of your work for you. You want to have a separation of concerns here.

Edit2 in response to comments

Comment: I have to update related records based on the status__c of the current chunk of records, approach (1) can I go directly to AfterInsert method and collect the ids and then process it using a helper class in AndFinally method or (2) collect up in bulkAfter and then process it using a helper class in AndFinally ?

You could do this in several different ways, including using a Helper Class & method linked to the BulkAfter section that selects AfterInsert insert records for addition to a list that gets processed in andFinally. Quite honestly, I think that would be a poor use of the helper method and muddies up the trigger. Instead, bulk all of your AfterInsert records, then send them directly to an AfterInsert Helper Class for sorting and further processing for update in the handler class.

I'd do something like this in the andFinally() method:

    if(trigger.isAfter && trigger.isInsert){
        SharingHandler EICS = new SharingHandler(newLst,oldLst,newMap,oldMap,trigger.isBefore,trigger.isAfter,trigger.isInsert,trigger.isUpdate);    
     }

My SharingHandler class's constructor (or one of them) will expect the two lists, two maps and 4 booleans. One can easily get away with only sending the two lists since you can readily rebuild the maps as follows:

map<id,sObjectName>oldMap = new map<Id,sObjectName>();
//  then doing:
oldMap.putAll(oldLst)

So, it isn't critical to pass the maps to the handler as they can easily be reconstructed without the need to iterate in a loop.

| improve this answer | |
  • Hi Cal, thanks for explaining Tony's pattern. I still don't fully understand it. Looking at your answer I can have DMLs & SOQLs only in andFinally method and not even in BulkBefore & BulkAfter methods. – Jarvis Jan 29 '16 at 18:06
  • Cal - I love it! - I wouldn't put the trigger context logic in the andFinally, rather, the context handlers should construct sets/lists/maps with good varaible names which, if non-empty, are dealt with in the andFinally – cropredy Jan 29 '16 at 19:22
  • 1
    @crop1645. Yup! That's one additional way of doing it too! I've been dealing with some really messy trigger handlers the last couple of weeks where there was way too much logic inside of the primary handler. I was trying to add my own to send records to my handlers and was having a difficult time avoiding messing up the existing logic! It just shouldn't be that way! =8-0 – crmprogdev Jan 29 '16 at 19:42
  • 1
    That's correct Benny. You can also take trigger.new in the andFinally method and cast it there to send to your helper class. Theres tons of ways of doing this. – crmprogdev Feb 6 '16 at 14:13
  • 1
    @bennie bulkafter is useful if after update or after insert handlers need related records to make decisions about what sobject a go into the collections used by andfinally – cropredy Feb 6 '16 at 17:21

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.