0

Hi i'm really new to Salesforce development in the following code I'm trying update existing record to inactive but, not the coming record on two different conditions. I'm a bit confused in the last bit of code where I'm doing nested for which could end up looping more than we expect. Can any one suggest me how best the code can enhanced to make it efficient. I'm using trigger for both before trigger and after trigger that's the oldCardRequestMap check is for.

public void checkRenewedCardsUpdateStatus(Map<Id, Card_Request__c> oldCardRequestMap, List<Card_Request__c> cardRequestList){
    //Card to be updated
    Map<Id ,Card_Request__c> cardsToBeUpdated = new Map<Id, Card_Request__c>();

    //Carer card renewal and replacements map
    Map<String, Card_Request__c> cardNumbersToCheckForRenewAndReplace = new Map<String, Card_Request__c>();


    Set<String> cardNumbers = new Set<String>();

    for(Card_Request__c card : cardRequestList){
        //renewed card
        if(oldCardRequestMap != null){
            Card_Request__c oldCard = oldCardRequestMap.get(card.Id);
            if((card.DateIssued__c != null && oldCard.DateIssued__c == null)||
               (card.Carer_Sub_Status__c == 'Sent' && oldCard.Carer_Sub_Status__c == 'Waiting - Print Receipt')||
               (card.Card_Status__c == 'Active' && oldCard.Card_Status__c == 'Issued')){
                cardNumbersToCheckForRenewAndReplace.put(card.Carers_Card_Number__c, card);
            }
        }
        else{
        //replacement card
            if(card.Carers_Card_Replacement_For__c != null){
                cardNumbersToCheckForRenewAndReplace.put(card.Carers_Card_Number__c, card);
            }
        }
    }
    //cardNumberToContactId.get(app.Existing_Card_Number__c) == null && appBlockStatuses.contains(app.Status__c))
    if(!cardNumbersToCheckForRenewAndReplace.isEmpty()){
        cardsToBeUpdated = new Map<Id, Card_Request__c>(); 
        List<Card_Request__c> oldCards = new List<Card_Request__c>();           
        for(Card_Request__c card : [SELECT Id, Card_Status__c, Carer_Sub_Status__c, SC_Contact__c, Carers_Card_Number__c, SC_Contact__r.Id, Carers_Card_Replacement_For__c
                        FROM Card_Request__c WHERE Card_Type__c = 'Carers Card' AND Carers_Card_Number__c IN :cardNumbersToCheckForRenewAndReplace.keySet()]){
            oldCards.add(cardNumbersToCheckForRenewAndReplace.get(card.Carers_Card_Number__c));
        }

        for(Card_Request__c newCard : cardRequestList){
            for(Card_Request__c oldCard : oldCards){
                if(newCard.Carers_Card_Number__c == oldCard.Carers_Card_Number__c 
                    && newCard.Id != oldCard.Id){
                    oldCard.Card_Status__c = 'Inactive';
                    cardsToBeUpdated.put(oldCard.Id ,oldCard);    
                }
            }
            update cardsToBeUpdated.values();
        }
    }
}
  • by way of clarification: is cardRequestList Trigger.new and oldCardRequestMap Trigger.oldMap ? – cropredy Sep 23 '18 at 4:21
  • Yes you are right – Mohammed Imran Sep 23 '18 at 5:39
  • Is it possible for one newcard to have more than one oldcard with the same Carers_Card_Number__c value? – Shane Steinfeld Sep 23 '18 at 17:33
1

Yes, I think you're on the right track: You can avoid the nested for loop by using a map.

First, change cardsToBeUpdated from a map to a list, unless you need to map from the card Id to the card record elsewhere. (The code below assumes you don't. If you do, it's no problem. You'll just need to refactor this a little bit.)

List<Card_Request__c> cardsToBeUpdated = new List<Card_Request__c>();

Then try this: For every new card, instead of looking at the whole list of old cards and trying to find a match, we're looking for the new card's number in a map (card Id to old card). If the new card's Id isn't in the key set, then we move on.

Map<String, Id> cardNumber_to_oldCard = new Map<String, Id>();

for(Card_Request__c oldCard : oldCards){
    cardNumber_to_oldCard.put(oldCard.Carers_Card_Number__c, oldCard);

}

for(Card_Request__c newCard : cardRequestList){

    Card_Request__c oldCard = cardNumber_to_oldCard.get(newCard.Carers_Card_Number__c);

    if(oldCard != null && 
       oldCard.Carers_Card_Number__c == newCard.Carers_Card_Number__c &&
       oldCard.Id != newCard.Id){

        oldCard.Card_Status__c = 'Inactive';
        cardsToBeUpdated.add(oldCard);

    }

}         
update cardsToBeUpdated;
| improve this answer | |

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.