0

what I should learn and do if I have 3 different methods that are essentially doing the same thing but only the query statement that makes the differences.

For instance my code below have 2 methods and do some SOQL,DML and Email send out, essentially both are doing exactly the same which is sending an email based on some conditions, in brief explanation about my code as follow;

2 methods do the specific query to get the desired result and then iterate through them all then in the iteration I put if statement like below

FOR(Contact cont : contacts){

                if(cont.Account.AccountCountry__c == 'ID') {

                    template = LostWinBackID;
                } 

                if(cont.Account.AccountCountry__c == 'MX') {

                    template = LostWinBackMX; 
                } 

                if(cont.Account.AccountCountry__c == 'PH'){

                    template = LostWinBackPH;
                }

then the next line of code will do the email sendout with pdf attachment based on the returned query, since both method take difference query so I decided to separate them into 2.

I wonder to write smart code to prevent repeated line of codes like this.

Looking forward to hearing from code experts.

public class automatedCommunicationNew{


    public List<OrgWideEmailAddress> orgWideMail = [SELECT Id FROM OrgWideEmailAddress];

    //Template for ID
    public EmailTemplate ExpiringContractID = [SELECT Id, Name, Description, Subject FROM EmailTemplate WHERE Name =: 'Contract renewal notice ID' LIMIT 1];
    public EmailTemplate DueInvoiceID = [SELECT Id, Name, Description, Subject FROM EmailTemplate WHERE Name =: 'Invoice due reminders ID' LIMIT 1];
    public EmailTemplate BlockingOutstandingID = [SELECT Id, Name, Description, Subject FROM EmailTemplate WHERE Name =: 'Invoice overdue ID' LIMIT 1];
    public EmailTemplate BlockingAccountID = [SELECT Id, Name, Description, Subject FROM EmailTemplate WHERE Name =: 'Account has been blocked ID' LIMIT 1];
    public EmailTemplate DeactivatingAccountID = [SELECT Id, Name, Description, Subject FROM EmailTemplate WHERE Name =: 'Account has been deactivated ID' LIMIT 1];
    public EmailTemplate LostWinBackID = [SELECT Id, Name, Description, Subject FROM EmailTemplate WHERE Name =: 'lost win back Id' LIMIT 1];

    //Template for MX
    public EmailTemplate ExpiringContractMX = [SELECT Id, Name, Description, Subject FROM EmailTemplate WHERE Name =: '9. Contract renewal notice' LIMIT 1];
    public EmailTemplate DueInvoiceMX = [SELECT Id, Name, Description, Subject FROM EmailTemplate WHERE Name =: '10. Invoice Due Reminder' LIMIT 1];
    public EmailTemplate BlockingOutstandingMX = [SELECT Id, Name, Description, Subject FROM EmailTemplate WHERE Name =: '11. Invoice overdue' LIMIT 1];
    public EmailTemplate BlockingAccountMX = [SELECT Id, Name, Description, Subject FROM EmailTemplate WHERE Name =: '12. Account has been blocked' LIMIT 1];
    public EmailTemplate DeactivatingAccountMX = [SELECT Id, Name, Description, Subject FROM EmailTemplate WHERE Name =: '13. Account has been Deactivated' LIMIT 1];
    public EmailTemplate LostWinBackMX = [SELECT Id, Name, Description, Subject FROM EmailTemplate WHERE Name =: 'lost win back mx' LIMIT 1];

    //template for PH
    public EmailTemplate ExpiringContractPH = [SELECT Id, Name, Description, Subject FROM EmailTemplate WHERE Name =: 'New Renewal Notice' LIMIT 1];
    public EmailTemplate DueInvoicePH = [SELECT Id, Name, Description, Subject FROM EmailTemplate WHERE Name =: 'Invoice Due in 2 Weeks' LIMIT 1];
    public EmailTemplate BlockingOutstandingPH = [SELECT Id, Name, Description, Subject FROM EmailTemplate WHERE Name =: 'Blocking Outstanding' LIMIT 1];
    public EmailTemplate BlockingAccountPH = [SELECT Id, Name, Description, Subject FROM EmailTemplate WHERE Name =: 'Blocking Account' LIMIT 1];
    public EmailTemplate DeactivatingAccountPH = [SELECT Id, Name, Description, Subject FROM EmailTemplate WHERE Name =: 'Deactivated Contract' LIMIT 1];
    public EmailTemplate LostWinBackPH = [SELECT Id, Name, Description, Subject FROM EmailTemplate WHERE Name =: 'lost win back Id' LIMIT 1];


    public Id emailtemplate;
    public String templateDescription;              // The template description will be assigned to the Task Subject of the task
    public String templateSubject;
    public Id invPDFattachmemt{get;set;}
    public Attachment attach{get;set;}
    public EmailTemplate template;
    public List<Invoice__c> linv;

    public String senderName = 'xxx';
    public String defaultReplyEmailID = 'test@test.com';
    public String defaultReplyEmailPH = 'test1@test.com';
    public String defaultReplyEmailMX = 'test2@test.com';

    public void sendRenewalReminder(){
        String ReplyToEmail;

/**** Set Template ****/

        List<Messaging.SingleEmailMessage> mails = new List<Messaging.SingleEmailMessage>();
        Set<Id> ids = new Set<Id>();

/**** Set filters and conditions (recipients and criteria -> To:, When?, who?, why?) ****/
        List<Contact> licontact = [SELECT Id FROM Contact
                            WHERE ContactForContracts__c = TRUE     // only contacts that are marked to receive contract info
                            AND Email <> ''
                            AND Account.ExcludeFromAutomatedSendouts__c = FALSE                 // Checkbox on account to exclude Account from sendouts
                            AND Account.Contractsigneduntil__c =: system.today() + 35           
                            AND 
                               (Account.AccountCountry__c = 'ID'
                            OR  Account.AccountCountry__c = 'PH'
                            OR  Account.AccountCountry__c = 'MX') 
                            AND Account.TypeOfCompany__c = 'Agent / Broker' 
                            AND (Account.LastRenewalNoticeDate__c <: system.today() OR Account.LastRenewalNoticeDate__c =: null)];  // avoids multiple sendouts in case the method is called several times on the same day
        system.debug('aaa' + licontact.size());


/**** Start process ****/

        IF(licontact.size() >0){
            /**** Set parameters ****/
            List<Contact> contacts = [SELECT Id, AccountId, Account.AccountManager__r.Email, Account.AccountManager__r.isActive, Account.Owner.Email, Account.Owner.isActive, Account.Contractsigneduntil__c
                                            FROM Contact WHERE Id in: licontact];
            FOR(Contact cont : contacts){
                ids.add(cont.Account.Id);
            }
            List<Account> accounts = [SELECT LastRenewalNoticeDate__c FROM Account WHERE Id in: ids];

            /**** Create emails ****/

            FOR(Contact cont : contacts){

                if(cont.Account.AccountCountry__c == 'ID') {

                    template = ExpiringContractID;
                } 

                if(cont.Account.AccountCountry__c == 'MX') {

                    template = ExpiringContractMX; 
                } 

                if(cont.Account.AccountCountry__c == 'PH'){

                    template = ExpiringContractPH;
                }

                emailtemplate = template.Id;
                templateDescription = template.Description;              // The template description will be assigned to the Task Subject of the task
                templateSubject = template.Subject; 

                Messaging.SingleEmailMessage mail = new Messaging.SingleEmailMessage();

                mail.setTemplateId(emailtemplate); 
                //mail.setSenderDisplayName(senderName);
                mail.setOrgWideEmailAddressId(orgWideMail[0].Id);
                mail.setTargetObjectId(cont.Id); 
                mail.setWhatId(cont.AccountId);

                /**** Set the email address to which the clients replies 
                      ** Aftersales manager by default
                          ** Account Owner if AS is inactive
                          ** default email address otherwise                    ****/
                IF(cont.Account.AccountManager__r.Email <> null && cont.Account.AccountManager__r.isActive == TRUE)
                   ReplyToEmail = cont.Account.AccountManager__r.Email;
                ELSE IF(cont.Account.Owner.Email <> null && cont.Account.Owner.isActive == TRUE)
                   ReplyToEmail = cont.Account.Owner.Email;
                ELSE IF(cont.Account.AccountCountry__c == 'ID')
                   ReplyToEmail = defaultReplyEmailID; 
                ELSE IF(cont.Account.AccountCountry__c == 'PH')
                   ReplyToEmail = defaultReplyEmailPH; 
                ELSE
                   ReplyToEmail = defaultReplyEmailMX;      
                mail.setReplyTo(ReplyToEmail);       

                mails.add(mail);
            }

            system.debug(mails.size());
            system.debug(mails);

        /**** Send emails ****/
            IF(mails.size() >0){
                Messaging.sendEmail(mails, false);

                /***** updates Account date of last sendout to avoid multiple sendouts ****/
                FOR(Account acc : accounts){
                    acc.LastRenewalNoticeDate__c = system.today();
                }

                update accounts;
            }
        }
    }

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////    

       public void sendLostWinBackEmails(){
        String ReplyToEmail;

/**** Set Template ****/

        List<Messaging.SingleEmailMessage> mails = new List<Messaging.SingleEmailMessage>();
        Set<Id> ids = new Set<Id>();

/**** Set filters and conditions (recipients and criteria -> To:, When?, who?, why?) ****/
        List<Contact> licontact = [SELECT Id FROM Contact
                            WHERE ContactForContracts__c = TRUE     // only contacts that are marked to receive contract info
                            AND Email <> ''
                            AND Account.ExcludeFromAutomatedSendouts__c = FALSE                 // Checkbox on account to exclude Account from sendouts         
                            AND Account.AccountCountry__c = 'ID' 
                            AND Account.TypeOfCompany__c = 'Agent / Broker' 
                            AND Account.LostSince__c <: system.today().addMonths(-3)
                            AND Account.SalesStage__c = '00. Lost Win Back'];  // avoids multiple sendouts in case the method is called several times on the same day
        system.debug('aaa' + licontact.size());


/**** Start process ****/

        IF(licontact.size() >0){
            /**** Set parameters ****/
            List<Contact> contacts = [SELECT Id, AccountId, Account.AccountManager__r.Email, Account.AccountManager__r.isActive, Account.Owner.Email, Account.Owner.isActive, Account.Contractsigneduntil__c
                                            FROM Contact WHERE Id in: licontact];
            FOR(Contact cont : contacts){
                ids.add(cont.Account.Id);
            }
            List<Account> accounts = [SELECT LastRenewalNoticeDate__c FROM Account WHERE Id in: ids];

            /**** Create emails ****/

            FOR(Contact cont : contacts){

                if(cont.Account.AccountCountry__c == 'ID') {

                    template = LostWinBackID;
                } 

                if(cont.Account.AccountCountry__c == 'MX') {

                    template = LostWinBackMX; 
                } 

                if(cont.Account.AccountCountry__c == 'PH'){

                    template = LostWinBackPH;
                }

                emailtemplate = template.Id;
                templateDescription = template.Description;              // The template description will be assigned to the Task Subject of the task
                templateSubject = template.Subject; 

                Messaging.SingleEmailMessage mail = new Messaging.SingleEmailMessage();

                mail.setTemplateId(emailtemplate); 
                //mail.setSenderDisplayName(senderName);
                mail.setTargetObjectId(cont.Id); 
                mail.setWhatId(cont.AccountId);
                mail.setOrgWideEmailAddressId(orgWideMail[0].Id);

                /**** Set the email address to which the clients replies 
                      ** Aftersales manager by default
                          ** Account Owner if AS is inactive
                          ** default email address otherwise                    ****/
                IF(cont.Account.AccountManager__r.Email <> null && cont.Account.AccountManager__r.isActive == TRUE)
                   ReplyToEmail = cont.Account.AccountManager__r.Email;
                ELSE IF(cont.Account.Owner.Email <> null && cont.Account.Owner.isActive == TRUE)
                   ReplyToEmail = cont.Account.Owner.Email;
                ELSE
                    ReplyToEmail = defaultReplyEmail;      
                mail.setReplyTo(ReplyToEmail);       

                mails.add(mail);

                if(Limits.getQueries() >= Limits.getLimitQueries()){
                    break;
                }
            }

            system.debug(mails.size());
            system.debug(mails);

        /**** Send emails ****/
            IF(mails.size() >0){
                Messaging.sendEmail(mails, false);

            }
        }
    }
}
1

I don't think it makes sense for me to analyze all your code and "optimize" it for you. I'll give you some ideas about what you could do to limit repeating yourself. This is not anything you can immediately copy and paste in your org, they're things you'll have to play around with and test on your own

  1. If the differences are only in queries, then you could store the query as a string (as they'd be static in your case it seems) and then pass a parameter in the class to understand which object/query you should be using (if(queryLogic = 'sendWinBack'{}).

https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_dynamic_soql.htm

Example

public static sendEmailProcess(String queryParam){

    //list all queries as a String that can be called depending on a parameter 
    you might pass
    String sendLostWinBack = 'SELECT Id FROM Contact WHERE ContactForContracts__c = TRUE AND Email <> \'\' AND Account.ExcludeFromAutomatedSendouts__c = FALSE AND Account.LostSince__c < LAST_90_DAYS....'//add rest of query here
    String renewalQuery = 'SELECT Id FROM Contact WHERE ContactForContracts__c = TRUE...';

    List<Contact> contactList = new List<Contact>();
    //I'm using an if statement to illustrate what you could do, 
    //but it could easily be a switch statement
    if(queryParam = 'LostWin'){
        contactList = Database.query(sendLostWinBack);
    } else if (queryParam = 'Renewal'){
        contactList = Database.query(renewalQuery);
    }
}
  1. Now, you know what's the repeat code. Break that out into methods that make sense and that are doing one thing. This makes it easy to unit test. In your case, one example could be
public List<Messaging.SingleEmailMessage> createEmailMessage(String templateName, String senderName, String replyToEmail){

//create your specific email based on as many unique parameters as you need. //Depending on what query you decided to use,you can pass specific info so 
//it can create emails for you that are unique for the given situation, 
//but are very similar in code. 


    Messaging.SingleEmailMessage mail = new Messaging.SingleEmailMessage();
    //query for template first based on parameter name
    mail.setTemplateId(emailtemplate); 
    mail.setSenderDisplayName(senderName);
    //rest of logic and return list of email or just send it within method

}
  • Hi Kris, thanks for your help, I have edited my code and added more details on my problem in hope the problem is more clear now – Adnan Zahry Nov 4 '19 at 6:38

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.