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
  • you may 'have created a map' per your OP but variable mapDelMaint is never declared – cropredy Nov 30 '14 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 '14 at 12:42
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.

  • Thanks @Eric, this makes a lot of sense. I will go with your example and fix up my help class. – jnoel10 Dec 1 '14 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 '14 at 14:31
  • @jnoel10 - It will not add a duplicate and that is exactly what a set if for. – Eric Dec 1 '14 at 21:08
  • Thanks, I will use set Only then instead of creating a list, then moving to a set – jnoel10 Dec 1 '14 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

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.