2
trigger MapAccountSource on Opportunity (after insert,after update) 
{
    Set<Id> opportunities_id = new Set<Id>();

    if(checkRecursive.runOnce())    //Make sure After update trigger does not go into recusrsive update
    {    
       if(trigger.isAfter)    //Make sure trigger executes only after insert & after update
       {
               for(Opportunity opt : trigger.new)           // trigger.new takes new inserted/updated opportunity records
               {    
                  System.debug('OPP IDS:'+ trigger.new);
                  opportunities_id.add(opt.id);            //Add Inserted/Updated Opportunity ID
               }
        }

       //This List get value of the Account Source Value from Related Account
       List<Opportunity> opportunities_records = [select Account_source__c,Account.AccountSource from Opportunity where id IN : opportunities_id];
       for(Opportunity opportunity : opportunities_records)
       {
           if(opportunity.Account.AccountSource!=null)    //Make sure Account Source is not Null
           {
               opportunity.Account_source__c = opportunity.Account.AccountSource;    //Assign value of Account Source to the field of opportunity
           }
       }
       update opportunities_records;    //updates Opportunity based on change of Relatd Account
   }
4
  • I am sure it can handle bulk records.But if you want to write in a efficient way use maps instead of lists.Like you write a query opportunities_records in a list use map instead.@Salesfor_dev
    – Pavan tej
    Jun 22, 2016 at 19:24
  • 1
    The code you've provided is already bulkified. Is there a specific problem that you're running into with this trigger?
    – Derek F
    Jun 22, 2016 at 19:28
  • @DerekF No, But i was not sure that it is bulkified or not, Thats why i am asking. Jun 22, 2016 at 19:28
  • 1
    This looks fine, your variable naming convention could be changed..
    – EricSSH
    Jun 22, 2016 at 20:38

1 Answer 1

2

Better can be a bit subjective.

There are a few changes that might improve it:

  1. If opportunities_id is empty there is no need to run SOQL queries on it. So move that code inside the trigger.isAfter clause (unless there is other code that adds Ids to it.)
  2. The first thing you do after the SOQL query is check if opportunity.Account.AccountSource is null. Why not express that in the SOQL query directly. That will reduce the number of records to retrieve and then ignore.
  3. There is a shorthand for extracting a Set of IDs - How can I efficiently generate a Set from a List structure?
  4. As an alternative to point 3 above, these isn't even a need to convert to a Set of Ids. You can actually do a SOQL query directly against a List of sObjects.

E.g. for point 4)

List<Account> accs = [Select Id, Name from Account limit 20];
List<Account> accsAgain = [Select Id from Account where Id in :accs];

Giving something like:

trigger MapAccountSource on Opportunity (after insert,after update) 
{
    //Make sure After update trigger does not go into recursive update
    if(checkRecursive.runOnce())    
    {    
       //Make sure trigger executes only after insert & after update
       if(trigger.isAfter)    
       {
           // Get the ID's of all the Opps as a Set
           Set<Id> opportunities_id = (new Map<Id,SObject>(trigger.new)).keySet();

           //This List get value of the Account Source Value from Related Account
           List<Opportunity> opportunities_records = 
               [select Account_source__c,Account.AccountSource 
                from Opportunity 
                where id IN :opportunities_id and Account.AccountSource != null];
           for(Opportunity opportunity : opportunities_records)
           {
               //Assign value of Account Source to the field of opportunity
               opportunity.Account_source__c = opportunity.Account.AccountSource;    

           }
           update opportunities_records;    //updates Opportunity based on change of Related Account


    }

    // ? ...

}

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .