4

I have a trigger on QuoteLineItem that I need to run some validations to figure out what Action I should do on my Quote.

a) if Product contains PREM or STND then i need to update field A on quote

b) if SP-DISC THEN I need to update field B on quote

c) if SP-DISC is not in the delete then I will need to see if it exists in the quote to delete it.

So, I created a map that has the QuoteID and a string that declares if both qli's are inDelete or just one(or none)

I find that the code looks dirty and is not as efficient as it could be. Does anyone have any recommendations on how to improve?

if(trigger.isDelete)
    {    
        Map<Id,String> mapDelMaint = new Map<Id,String>();
        List<QuoteLineItem> lDelQLI = new List<QuoteLineItem>();
        List<Id> lDelQuote = new List<Id>();
        Set<Id>sDelQuote = new Set<Id>();
        for(QuoteLineItem qli:Trigger.Old)
        {
            QuoteLineItem findMaint = new QuoteLineItem();
            if(qli.Product_Code__c == 'SP-DISC-MAINT')
            {                
                lDelQLI.add(qli);
                lDelQuote.add(qli.QuoteId);
            }
            else if(qli.Product_Code__c.substring(2,6) == 'PREM' || qli.Product_Code__c.substring(2,6) == 'STND')
            {
                lDelQLI.add(qli);
                lDelQuote.add(qli.QuoteId); 
            }
        }//END OF FOR QUOTELINEITEM QLI

        //To only have unique QuoteIds 
        sDelQuote.addAll(lDelQuote);
        lDelQuote.clear();
        lDelQuote.addAll(sDelQuote);
        sDelQuote.clear();

        for(Id i: lDelQuote)
        {
            Boolean bRemoveMaint = FALSE;
            Boolean bRemoveSpec  = FALSE;
            for(QuoteLineItem qli : lDelQLI)
            {
                if(qli.Product_Code__c == 'SP-DISC-MAINT')                                                    
                    bRemoveSpec = TRUE;       
                else if(qli.Product_Code__c.substring(2,6) == 'PREM' || qli.Product_Code__c.substring(2,6) == 'STND')                          
                    bRemoveMaint = TRUE;               
            }
            if(bRemoveMaint == TRUE && bRemoveSpec == TRUE)            
                mapDelMaint.put(i,'BOTH');            
            else if(bRemoveMaint == TRUE)
                mapDelMaint.put(i,'MAINT');
            else if(bRemoveSpec == TRUE)
                mapDelMaint.put(i,'SPECIAL');
        }

        if(mapDelMaint.size()>0)            
            class_QLI_UpdateMaintenanceAmount.removeMaintenance(mapDelMaint);                       
    }//END OF isDelete
2
  • you may 'have created a map' per your OP but variable mapDelMaint is never declared
    – cropredy
    Nov 30, 2014 at 23:40
  • Yes sorry, I declared it above the if(trigger.isDelete) I should have brought it down in the example.
    – jnoel10
    Dec 1, 2014 at 12:42

2 Answers 2

1

I would build it differently but it would require you to update the helper class. I am assuming that the helper class is not as efficient as it could be either. In addition, I find all the boolean values to be unnecessary complication to the code....

You will need to update the helper class to use Map<String,Set<ID>> but this will allow you to eaisly process based on the Key as the Key is the function SPECIAL or MAINT or BOTH.

if(trigger.isDelete)
    {    
        Set<ID> quote_ids = New Set<ID>();


        Map<String,Set<ID>> mapDelMaint = New Map<String,Set<ID>>{
                                                        'BOTH'=>New Set<ID>(),
                                                        'MAINT'=>New Set<ID>(),
                                                        'SPECIAL'=>New Set<ID>(),
                                                        };

        //Populate Function Map
        for(QuoteLineItem qli:Trigger.Old)
        {
            //Populate Set of Quote IDs being processed
            quote_ids.add(qli.QuoteID);

            if(qli.Product_Code__c == 'SP-DISC-MAINT')
            {                
                mapDelMaint.get('SPECIAL').add(qli.QuoteID);
            }
            else if(qli.Product_Code__c.substring(2,6) == 'PREM' || qli.Product_Code__c.substring(2,6) == 'STND')
            {
                mapDelMaint.get('MAINT').add(qli.QuoteID);
            }
        }//END OF FOR QUOTELINEITEM QLI

        //Now loop through and see if they exist in both and move them

        for(Id i: quote_ids){

            if( mapDelMaint.get('SPECIAL').contains(i) && mapDelMaint.get('MAINT').contains(i) ){
                mapDelMaint.get('BOTH').add(i);
                mapDelMaint.get('MAINT').remove(i);
                mapDELMaint.get('SPECIAL').remove(i);
            }

        }


        //if(mapDelMaint.size()>0) // I would move this to your class and check there for each function
            class_QLI_UpdateMaintenanceAmount.removeMaintenance(mapDelMaint);                       
    }//END OF isDelete

Doing it this way you end up with a set of IDs to:

BOTH MAINT SPECIAL

Your helper class can then process accordingly.

4
  • Thanks @Eric, this makes a lot of sense. I will go with your example and fix up my help class.
    – jnoel10
    Dec 1, 2014 at 12:43
  • I am sure I will figure this out by testing, but if you add the same Id in a SET will it create a duplicate? Meaning, if you did set.Add('123'), set.Add('234'), Set.add('123'). for(set i) ... will it loop 3 time or just twice?
    – jnoel10
    Dec 1, 2014 at 14:31
  • @jnoel10 - It will not add a duplicate and that is exactly what a set if for.
    – Eric
    Dec 1, 2014 at 21:08
  • Thanks, I will use set Only then instead of creating a list, then moving to a set
    – jnoel10
    Dec 1, 2014 at 22:01
1

Here is my stab at this. You can use some built in map methods to help reduce the amount of work being done: map.containsKey and map.remove. This should allow you to reduce the second loop and the adding to set and back to list portions. Let me know your thoughts.

if(trigger.isDelete)
{    
    Map<Id, String> mapDelMaint = new Map<Id, String>();

    for(QuoteLineItem qli:Trigger.Old)
    {
        Boolean bRemoveSpec  = FALSE;
        Boolean bRemoveMaint = FALSE;

        if(qli.Product_Code__c == 'SP-DISC-MAINT')
        {
            if(!mapDelMaint.containsKey(qli.QuoteId))
                apDelMaint.put(i,'SPECIAL');

            bRemoveSpec = TRUE;
        }
        else if(qli.Product_Code__c.substring(2,6) == 'PREM' || qli.Product_Code__c.substring(2,6) == 'STND')
        {
            if(!mapDelMaint.containsKey(qli.QuoteId))
                mapDelMaint.put(i,'MAINT');

            bRemoveSpec = TRUE;
        }

        if(bRemoveSpec && bRemoveMaint)
        {
            mapDelMaint.remove(qli.QuoteId);
            mapDelMaint.put(qli.QuoteId, 'BOTH')
        }
    }//END OF FOR QUOTELINEITEM QLI

    if(mapDelMaint.size()>0)            
        class_QLI_UpdateMaintenanceAmount.removeMaintenance(mapDelMaint);                       
}//END OF isDelete

You must log in to answer this question.

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