Use Map instead of For loop in my Apex Trigger [duplicate]

Question

I'm new to coding and I need help in modifying the apex trigger below to the correct one. Can you also help me in creating a test class for this? Thank you so much!

trigger UpdateLadderedFields on OpportunityLineItem (before insert, before update) 
{
   for (OpportunityLineItem oli: Trigger.new)
   {
      string OppprodSKUID;
      OppprodSKUID = oli.SKU_ID__c.substring(0,15);
      
      If(oli.Q4_Laddered__c != null)
      {
         List<Ladders_RDP__c> lr = [SELECT ID, SKU_ID__c, Total_Laddered__c, OE_Qtr__c
                                       FROM Ladders_RDP__c WHERE SKU_ID__c = :OppprodSKUID];
         for(Ladders_RDP__c l: lr)
         {
             If(l.OE_Qtr__c.contains('Q4')) {
                 oli.Q4_Laddered__c = l.Total_Laddered__c;}
             else If(l.OE_Qtr__c.contains('Q3')){
                 oli.Q3_Laddered__c = l.Total_Laddered__c;}
             else If(l.OE_Qtr__c.contains('Q2')){
                 oli.Q2_Laddered__c = l.Total_Laddered__c;}
             else IF(l.OE_Qtr__c.contains('Q1')){
                 oli.Q1_Laddered__c = l.Total_Laddered__c;}
         }
      }
   }
}

Answer 1

As a general rule, you should try to pre-fetch the data before entering the loop.

Re-writing a bit of your code, I will get this. I have not tested it, but you will get the general idea. Of course you need to bulk test this for your exact business scenario.

trigger UpdateLadderedFields on OpportunityLineItem (before insert, before update) 
{
    Map<String,OpportunityLineItem> oppLineItemSKUIDMap = new Map<String,OpportunityLineItem>();

    for (OpportunityLineItem oli: Trigger.new)
    {
        string OppprodSKUID;
        OppprodSKUID = oli.SKU_ID__c.substring(0,15);
        oppLineItemSKUIDMap.put(OppprodSKUID,oli);

    }

    List<Ladders_RDP__c> lr = [SELECT ID, SKU_ID__c, Total_Laddered__c, OE_Qtr__c
                                FROM Ladders_RDP__c WHERE SKU_ID__c in:oppLineItemSKUIDMap.keySet()];

    for(Ladders_RDP__c l: lr){
        String skuId = l.SKU_ID__c;
        OpportunityLineItem oli = oppLineItemSKUIDMap.get(skuId);
        If(oli.Q4_Laddered__c != null)
        {
            If(l.OE_Qtr__c.contains('Q4')) {
                oli.Q4_Laddered__c = l.Total_Laddered__c;}
            else If(l.OE_Qtr__c.contains('Q3')){
                oli.Q3_Laddered__c = l.Total_Laddered__c;}
            else If(l.OE_Qtr__c.contains('Q2')){
                oli.Q2_Laddered__c = l.Total_Laddered__c;}
            else IF(l.OE_Qtr__c.contains('Q1')){
                oli.Q1_Laddered__c = l.Total_Laddered__c;}
        }
    }

}

As for your test method, you would need to just insert the Ladders_RDP__c, Opportunity and OpportunityLineItem and use Asserts to validate the values. If you can begin a simple test and post it, perhaps I can help fix it.