0

I have a requirement that after inserting a record in Campaign Account(Custom Object lookup with Account) it should check the Campaign Account Name with opportunities related to the Account. I was able to achieve the functionality using the trigger. But I wanted to optimize my code so can someone help me with my code.

//Helper for Trigger

public class CreateOppAfterCAInsertionHelper {
    **public string name;**
    **public Id accId;**
    public void insertOpp(List<Campaign_Account__c> campaignAccList) {
        Set<Id> accIds = new Set<Id>();
        List<Opportunity> oppList = new List<Opportunity>();
        List<Opportunity> updatedOpps = new List<Opportunity>();
        for(Campaign_Account__c campAcc : campaignAccList) {
            if(campAcc.Account__c != NULL) {
                accIds.add(campAcc.Account__c);
                **name = campAcc.Name;**
                **accId = campAcc.Account__c;**
            }
        }
        oppList = [SELECT Name, AccountId, Count__c 
                   FROM Opportunity 
                   WHERE AccountId IN:accIds AND Name =: name];
        if(oppList.size() > 0) {
            for(Opportunity opp : oppList) {
                opp.Count__c  = opp.Count__c + 1;
                updatedOpps.add(opp);
            }
        }
        if(oppList.size() == 0) {
            Opportunity newOpp = new Opportunity();
            newOpp.Name = name;
            newOpp.StageName = 'Prospecting';
            newOpp.CloseDate = date.today();
            newOpp.AccountId = accId;
            insert newOpp;
        }
        if(updatedOpps.size() > 0) {
            update updatedOpps;
        }
    }
}

I want to avoid the highlighted lines. Need help thank you...

3
  • This code will not yield correct results if multiple Campaign Accounts are inserted in the same transaction that require the creation of Opportunities.
    – David Reed
    Mar 14, 2021 at 5:15
  • Would you explain me the reason why and how to resolve it...
    – user93540
    Mar 14, 2021 at 5:17
  • Why don't you give it a try hands-on? You can test the behavior with some Anonymous Apex and try to rewrite your logic (you need to be prepared to handle each parent Account separately!), which should also involve removing the lines you don't like. If you hit a problem, you can edit your question to get some more specific help.
    – David Reed
    Mar 14, 2021 at 5:19

1 Answer 1

0

I'm not sure I fully understand your intent, but I've closely modeled what you've already built, but with bulkification. Comments are included rather than try to explain the entire thing in a few paragraphs, as it's already quite large.

Note that checking if a list is empty before inserting, updating, iterating, etc, is generally not necessary. There may be some very minor CPU advantage in doing so, but you're writing unnecessary code. There's a reason why there are so few if statements in this code.

//Helper for Trigger

public class CreateOppAfterCAInsertionHelper {
    public void insertOpp(List<Campaign_Account__c> campaignAccList) {
        // Various data structures we want to use to keep track of existing/new opportunities
        Map<Id, Map<String, Opportunity> existingOpportunities = new Map<Id, Map<String, Opportunity>>();
        Map<Id, Opportunity> updatedOpportunities = new Map<Id, Opportunity>();
        Map<Id, Map<String, Opportunity> newOpportunities = new Map<Id, Map<String, Opportunity>();
        Opportunity[] newOpportunitiesToInsert = new Opportunity[0];

        // Names and Ids we want to query
        Set<Id> accountIds = new Set<Id>();
        Set<String> campaignNames = new Set<String>();

        // Initialize our data collection structures
        for(Campaign_Account__c record: campaignAccList) {
            if(record.Account__c == null) {
                continue;
            }
            accountIds.add(record.Account__c);
            existingOpportunities.put(record.Account__c, new Map<String, Opportunity>());
            newOpportunities.put(record.Account__c, new Map<String, Opportunity>());
            campaignNames.add(record.Name);
        }
        // Query existing opportunities and place them in Accountid -> Name manner.
        // We're matching the Campaign Account to the account's Opportunity by the same name.
        for(Opportunity record: [SELECT Name, AccountId, Count__c FROM Opportunity WHERE Name IN :campaignNames AND AccountId IN :accountIds]) {
            existingOpportunities.get(record.AccountId).put(record.Name, record);
        }

        for(Campaign_Account__c record: campaignAccList) {
            if(record.Account__c == null) {
                continue;
            }
            Opportunity existingOpportunity = existingOpportunities.get(record.Account__c).get(record.Name);
            // There is no existing opportunity
            if(existingOpportunity == null) {
                existingOpportunity = newOpportunities.get(record.Account__c).get(record.Name);
                // We're also not already inserting an opportunity, so initialize it now
                if(existingOpportunity == null) {
                    existingOpportunity = new Opportunity(Name=record.Name, StageName='Prospecting', CloseDate=Date.today(), AccountId=record.Account__c, Count__c=0);
                    newOpportunities.get(record.Account__c).put(record.Name, existingOpportunity);
                }
            } else {
                // We will be updating this opportunity's count value
                updatedOpportunities.put(existingOpportunity.Id, existingOpportunity);
            }
            existingOpportunity.Count__c += 1;
        }

        // Populate the final DML list for insertion
        for(Map<String, Opportunity> newOpportunityMap: newOpportunities.values()) {
            newOpportunitiesToInsert.addAll(newOpportunityMap.values());
        }

        insert newOpportunitiesToInsert;
        update updatedOpportunities.values();
    }
}
1
  • Thanks for your help. I've learnt bulkification by referencing your code...
    – user93540
    Mar 14, 2021 at 10:27

You must log in to answer this question.