Keeping Apex DRY, because no one likes soggy Apex

10/11/2012

Keeping Apex dry huh? This may be the first thing that comes to mind…

While this is a magnificent piece of MS Paint art this post will actually address a different kind of dry. DRY to be precise, or Don’t Repeat Yourself, and how this relates to Apex code on the force.com/salesforce.com platform. In addition to keeping things DRY I am also a religious believer in the KISS principle (Keep It Simple Stupid), and YAGNI (You ain’t gonna need it) to round off the acronym smorgasbord. With all of this in mind the content of this post may not be the “best” way to do something, and is surely not the only way, but it is definitely a stupidly simple and DRY way to design and architect Apex code. If you have been programming for years this is probably basic stuff but if you are new to Apex, or programming in general, this should be perfect for you.

Let’s pretend we have a database with thousands or maybe even million of Contact records. Now let’s pretend there is a business rule in place where the owner of all Contact records must be the same as the Account Owner. Ah ha! Let’s do this with a trigger! Yes, let’s! Here is what this bad boy trigger might look like.

trigger updateOwner on Contact (before insert, before update) {
 
    //Create and populate a list of Account Ids the contacts are related to
    Set<Id> acctIds = new Set<Id>();
    for(Contact con : trigger.new){
        acctIds.add(con.AccountId);
    }
 
    //Query the related account owners and put in may where key is account Id, value is owner Id
    Map<Id,Id> accountOwnerMap = new Map<Id,Id>();
    for(Account acct : [select Id, OwnerId from Account where Id IN : acctIds]){
        accountOwnerMap.put(acct.Id,acct.OwnerId);
    }
 
    //Set the owner Id field on the account, only do this if they are different
    for(Contact con : trigger.new){
        if(con.OwnerId != accountOwnerMap.get(con.AccountId) ){
            con.OwnerId = accountOwnerMap.get(con.AccountId); 
        }
    }
}

Looks good right? Well…it’s okay, but we can make it better. The main issue is the logical code is inside the trigger itself and it can’t be reused anywhere, like from a Visualforce page or a Batch Apex. So Rule #1, Don’t put logical code in a trigger. A better way to structure this would be to put the change owner logic in a separate class and call this class method from the trigger.

I have a confession to make. I have broken Rule #1 a lot. I can already hear the cries of outrage but hear me out. Sometimes I know for a fact that a trigger’s code is so unique to the problem it is solving it will never be reused anywhere else. For triggers this happens a lot as they are usually in place to address a very specific need that could not be addressed in the declarative areas of the platform. Part of what I am about to say goes back to me being a crazy KISS believer. Why should I put my code in another class just because this is the “right” way to do it even if there is no additional benefit? It adds complexity, albeit minor, and it also unnecessary bloats the number of classes. Often I have a trigger and a test class, that’s it. If I have a trigger, class for code, and a test class I’ve now increased the number of classes in my org by a third with no real benefit. So maybe…(wait, putting on flame suite) Rule #1 should be, Don’t put logical code in a trigger if you think there is a chance it could be reused somewhere else.

Usually code is going to be reused so that rant was much to do about nothing and this example is no different. This code could totally be reused so we will move the logical code to it’s own class called ContactAssignment which includes a method called changeOwner() and this method will change the owner field on the contact.

public Class ContactAssignment{
 
    public static void changeOwner(List<Contact> contacts){
 
        //Create and populate a list of Account Ids the contacts are related to
        Set<Id> acctIds = new Set<Id>();
        for(Contact con : contacts){
            acctIds.add(con.AccountId);
        }
 
        //Query the related account owners and put in may where key is account Id, value is owner Id
        Map<Id,Id> accountOwnerMap = new Map<Id,Id>();
        for(Account acct : [select Id, OwnerId from Account where Id IN : acctIds]){
            accountOwnerMap.put(acct.Id,acct.OwnerId);
        }
 
        //Set the owner Id field on the account, only do this if they are different
        for(Contact con : trigger.new){
            if(con.OwnerId != accountOwnerMap.get(con.AccountId) ){
                con.OwnerId = accountOwnerMap.get(con.AccountId); 
            }
        }
    }
}

Next we need to update the trigger to utilize this new class and method.

trigger updateOwner on Contact (before insert, before update) {
 
    //Call the changeOwner() method in the ContactAssignment class and pass over the records being processed by this trigger.
    ContactAssignment.changeOwner(trigger.new); 
 
}

At this point the trigger now behaves the same but our logical code has been removed from the trigger and can be utilized from many different places within Apex.

In a perfect word Apex triggers will fire all the time with every CRUD operation and we would have no issues with the current setup but, I hate to break it to you, we do not live in a perfect world. Sometimes Apex Triggers won’t always be successful due to user permission issues, validation rules, code bugs (your’s and salesforce.com’s), and who knows what else. Maybe in our org this trigger works perfectly but let’s again pretend we are super paranoid about this business rule and it is critical that all contact owners match account owners. So in addition to this trigger we also want a nightly Batch Apex job that queries and updates all contacts and changes the owner as necessary.

Here is what the batch job looks like and this brings us to Rule #2, a Batch Apex class should never included any of the code/business logic, it should only query the records and send them somewhere else for processing. Looking at the class below we are doing just this. The only thing this batch class does is query the records and then sends them to the ContactAssignment class for processing. The same class and method used by the trigger.

global class ContactAssignmentBatch implements Database.batchable<sObject>{
 
    String query = 'select OwnerId, AccountId from Contact';
 
    global Database.QueryLocator start(Database.BatchableContext bc){ 
        return Database.getQueryLocator(query);
    }    
 
    global void execute(Database.BatchableContext bc, List<Contact> contacts){
        //Send queried contacts to the ContactAssignment class for processing
        ContactAssignment.changeOwner(contacts);
    }     
 
    global void finish(Database.BatchableContext info){   
        system.debug('All done. Hooray!');  
    } 
}

There is one very important change we need to make to the ContactAssignment.changeOwner() method. In the context of a trigger when you edit a record the code doesn’t need to perform any explicit update on the record. As the records are going through the normal save workflow and any changes you make to the record in the code will be reflected after save. Calling this method from batch apex is totally different. We are passing records over to the class to be processed that are outside of the normal save workflow, where a trigger would normally execute, and therefore we must update the contact records our self with an actual update statement. For this class to work in both trigger and non-trigger contexts we have to make some changes. The comments in the code should explain these.

public Class ContactAssignment{
 
    public static void changeOwner(List<Contact> contacts){
 
        //Create and populate a list of Account Ids the contacts are related to
        Set<Id> acctIds = new Set<Id>();
        for(Contact con : contacts){
            acctIds.add(con.AccountId);
        }
 
        //Query the related account owners and put in may where key is account Id, value is owner Id
        Map<Id,Id> accountOwnerMap = new Map<Id,Id>();
        for(Account acct : [select Id, OwnerId from Account where Id IN : acctIds]){
            accountOwnerMap.put(acct.Id,acct.OwnerId);
        }
 
        //Set the owner Id field on the account
        List<Contact> consToUpdate = new List<Contact>(); //This is new.
        for(Contact con : contacts){
 
            if(con.OwnerId != accountOwnerMap.get(con.AccountId) ){
                con.OwnerId = accountOwnerMap.get(con.AccountId); //If being processed by the trigger this will work fine but....
 
                /*...if not being processed by a trigger such as batch job or a future we need 
                to explicitly update the records, first step is adding them to a list*/
                consToUpdate.add(con);
            }
        }
 
        //If we are not currently within the context of a trigger (batch, @future, etc) we need to update the records
        if(Trigger.isExecuting == false && contacts.size() > 0){
            update consToUpdate;
        }
    }
}

At this point our code is nice and DRY. All of our logic is in once place and this is good for many reasons. If we need to change the reassignment logic we only have to do it one place and our batch class and trigger don’t need to be touched. It’s also easier to test and debug because all the code is is one place and not duplicated within the system.

Now let’s make this interesting. Let’s pretend (pretending is fun right?) we have too many triggers on the Contact and Account objects and we are starting to hit limit errors such as too many query rows or too many DML statements. After looking at all the triggers on these two objects we realize the changeOwner trigger doesn’t need to be real time. What we can do is change the code in the in the ContactAssignment class to fire asynchronously or not real time. The salesforce.com system will queue this job up and run it when system resources become available. You can read more about @future and asynchronous jobs here. Easy, just add the @future keyword to the changeOwner() method and we are good…right? Something like this:

public Class ContactAssignment{
 
    @future //This is new, but bad.
    public static void changeOwner(List<Contact> contacts){
 
        //Create and populate a list of Account Ids the contacts are related to
        Set<Id> acctIds = new Set<Id>();
 
        //Same as above...
}

Unfortunately it’s not this simple as our Batch Apex job will completely implode and fail the next time it tries to run. Batch Apex jobs are also considered an asynchronous operation and you can’t call one async job from another; future to future, batch to future, future to batch, etc.

What we need to do is create a special method in the class that can handle the @future request, prepare the records that need to be processed, and then sends these records to the changeOwner() method that we use in the trigger and use from the Batch Apex class. This is what it might look like. First the modified ContactAssignment class and then the trigger.

public Class ContactAssignment{
 
    @future
    public static void changeOwnerFuture(Set<Id> contactIds){ //We can't pass the list of objects, we can only pass the contact Ids
        //Since all we have are the IDs we actually need to query the contact records we are going to process
        List<Contact> contacts = [select OwnerId, AccountId from Contact where Id IN :contactIds];
 
        //Now send these contact records over to the main method for processing
        ContactAssignment.changeOwner(contacts);
    }
 
    public static void changeOwner(List<Contact> contacts){ 
        //same as above, this is where all our core logic is
    }
}

In the trigger there is another thing we have to worry about, the infamous infinite loop, dun dun DUNNNNNNNNN! I actually discussed this in a previous post and it required creating your own static variables to fix this. Since then salesforce.com has made addressing this issue much easier. It’s also not really an infinite loop and the issue really boils down to calling an @future method in the context of an @future execution, a no no. Fortunately there is now a system method to check for this and it’s in the trigger below.

trigger updateOwner on Contact (before insert, before update) {
 
    //Only call the future method if we are not currently processing a future or batch job
    if(system.isFuture() == false && system.isBatch() == false){
 
        //Call the changeOwnerFuture() method in the ContactAssignment class and pass over the records IDs being processed by this trigger.
        ContactAssignment.changeOwnerFuture(trigger.newMap.keySet()); //Neat little trick to pass record Ids
    }
}

The comments in the code explain a lot but here are some things to note. When you call an @future method you can only pass over primitive datatypes like Id, Strings, etc so in our special @future changeOwnerFuture() method we actually need to query the records to process as they are not automatically passed over. Then we send these records to the changeOwner() method and all is good!

Nothing I’ve talked about in this post is mind blowing amazing (except for maybe the art) but rather focuses on some core design guidelines to think about when working with Apex code, or any programming language for that matter, and keeping it DRY. Like I said before what I’ve laid out here is just one way to accomplish this and there are ways to make what I have done here even better. Please add comments below if you have other insights.

Oh, and if you are one of the people that is like, “Uh ya, your @future implementation is totally lame because you could like totally actually not execute two queries and that is such a waste n00b”, you are absolutely correct, and we need to keep in touch. :-)