0

I had to add some business logic to our code base. I am trying to follow a "loosely coupled" architecture. So I was able to design the code where my trigger calls a class which does some business processing. Now in the class, I have added more business logic and my goal was to remove this logic and put it another class' method. This is what I have so far: OpportunityTrigger:

trigger OpportunityTrigger on Opportunity ( before insert, 
                                                before update,
                                                before delete,
                                                after insert,
                                                after update,
                                                after delete,
                                                after undelete) 
{
if( Trigger.isAfter && Trigger.isUpdate )
    {
        OpportunityHandler.onAfterUpdate( Trigger.new , Trigger.oldMap );
    }
}

OpportunityHandler:

public class OpportunityHandler {
 public static void onAfterUpdate( List<Opportunity> a_opportunities , Map<ID,Opportunity> l_old_map )
    {
      Map<Id, Opportunity> ratingMap = new Map<Id, Opportunity>();
      for(Opportunity opp: a_opportunities) { 
            //Only for Preschool Opportunities
            if (lppRecordTypeIds.contains(opp.RecordTypeId)) { 
                if(opp.LPP_Rating__c != null && l_old_map.get(opp.Id).LPP_Rating__c == null && opp.Collection_Status__c <> 'Open') {
                    ratingMap.put(opp.accountId, opp);
                }
}
      if(!ratingMap.isEmpty()) {
            system.debug('******************************************* we are calling the setAccountSeasonalRating method');
            LPP_OpportunityHandler.setAccountSeasonalRating(ratingMap);
        }
    }

}

LPP_OpportunityHandler:

public with sharing class LPP_OpportunityHandler 
{
   public static void setAccountSeasonalRating(Map<Id, Opportunity> ratingMap) {
        List<Account> recordsToUpdate = new List<Account>();
        if(ratingMap.size() > 0) {
            for(Account a : [select Id, Fall_Rating__c, Spring_Rating__c from Account where Id in: ratingMap.keySet()]) {
                if(ratingMap.get(a.Id).Season__c == 'Fall') {
                    a.Fall_Rating__c = ratingMap.get(a.Id).LPP_Rating__c;
                    recordsToUpdate.add(a); 
                }
                else if(ratingMap.get(a.Id).Season__c == 'Spring') {
                    a.Spring_Rating__c = ratingMap.get(a.Id).LPP_Rating__c;
                    recordsToUpdate.add(a); 
                }

                if(a.Spring_Rating__c !=null || a.Fall_Rating__c !=null){
                  //This part of the if/else below is what I wanted to cut out and add in another class. I was thnking of doing something like Lp_OppoortunityHandlerUtilities.ratungAverageCalcuTOR(a.Spring_Ratng, fallrating) but was not sure what parameters would need to be passed and how the new method in a different class would take the parameters. 
                    if(a.Fall_Rating__c == 'A' && a.Spring_Rating__c =='A'){
                        a.Rating__c = 'A'
                        recordsToUpdate.add(a);
                    }  
                    else if(a.Fall_Rating__c == 'A' && a.Spring_Rating__c =='B'){
                        a.Rating__c = 'B'
                        recordsToUpdate.add(a);
                    }
                    else if(a.Fall_Rating__c == 'A' && a.Spring_Rating__c =='C'){
                        a.Rating__c = 'B'
                        recordsToUpdate.add(a);
                    }
}

            }
        }
        if(recordsToUpdate.size() > 0) {
            update recordsToUpdate;
        }

}

In the opportuityhandler class, I have made a comment as to where I want the logic taken out. I wanted to take the new if/else statment that I have added and replace it with a method that is being called. This new method would have the logic. I was not sure how to do it and what parametrs would be passed in the method itself and when its being called. So the above is code what I have currently. This is my attempt in breaking it down (all other classes/triggers are same. Just showing what has changed):

OpportunityHandler:

public static void setAccountSeasonalRating(Map<Id, Opportunity> ratingMap) {
        List<Account> recordsToUpdate = new List<Account>();
        if(ratingMap.size() > 0) {
            for(Account a : [select Id, Fall_Rating__c, Spring_Rating__c from Account where Id in: ratingMap.keySet()]) {
                if(ratingMap.get(a.Id).Season__c == 'Fall') {
                    a.Fall_Rating__c = ratingMap.get(a.Id).LPP_Rating__c;
                    recordsToUpdate.add(a); 
                }
                else if(ratingMap.get(a.Id).Season__c == 'Spring') {
                    a.Spring_Rating__c = ratingMap.get(a.Id).LPP_Rating__c;
                    recordsToUpdate.add(a); 
                }
                if(a.Spring_Rating__c !=null || a.Fall_Rating__c !=null){
                    a.Rating__c = LPP_OpportunityHandlerUtilities.calculateAverageAccountRating(a.Spring_Rating__c, a.Fall_Rating__c)
                    recordsToUpdate.add(a_rating)
                    //everything here was moved to a diiferent class method.


                }

            }
        }
        if(recordsToUpdate.size() > 0) {
            update recordsToUpdate;
        }

    }

New class created OpportunityHandlerUtilities:

public without sharing class LPP_OpportunityHandlerUtilities{
public static String calculateAverageAccountRating(String Fall_Rating, String Spring_Rating){
        String Rating;
        if(Fall_Rating == 'A' && Spring_Rating =='A'){
            Rating = 'A';
            return Rating;
        }   
        else if(Fall_Rating == 'A' && Spring_Rating =='B'){
            Rating = 'B';
            return Rating;
        }
        else if(Fall_Rating == 'A' && Spring_Rating =='C'){
            Rating = 'B';
            return Rating;
        }
}
}
  • 1
    In its current form, I don't see an issue. However if you want to take that snippet out, there could be possibly different ways to handle this, have you tried anything specifically yet? – Jayant Das Apr 9 at 14:46
  • 1
    I dont think you should write businss logic in trigger handler's onAfterUpdate method. onAfterUpdate should only call the service methods – Pranay Jaiswal Apr 9 at 14:55
  • Hi @Pranay Jaswal, you are right thats why the onAfterUpdate is calling a different method that has the logic. The trigger has no business logic. – user3688231 Apr 9 at 14:59
  • @Jayant Das. I will post my attempt(what I am trying to accomplish), shortly. Maybe then you can point the flaws and logic in the change. But you are right. Right now it works fine. I just dont want the new logic in there. I want to call a method that will hold the new logic. – user3688231 Apr 9 at 15:00
  • @jayant Das I have added my attempt for this. Thanks – user3688231 Apr 9 at 21:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.