2

In the following nested for loop in getting duplicate id error. I think it's looping more than once and adding records again. Anyone please suggest something to enhance this code to avoid over looping

public void checkRenewedCardsUpdateStatus(Map<Id, Card_Request__c> oldCardRequestMap, List<Card_Request__c> cardRequestList){
    System.debug('executing method');
    //ist<Card_Request__c> cardsToCheckForRenewals = new List<Card_Request__c>();
    List<Card_Request__c> existingRenewalCardsRequests = new List<Card_Request__c>();
    Map<Id, Card_Request__c> cardNumbersToCheckForRenewals = new Map<Id, Card_Request__c>();
    for(Card_Request__c card : cardRequestList){
        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'){
            System.debug('Found Updating card');
            System.debug(card);
            cardNumbersToCheckForRenewals.put(card.SC_Contact__c,card);

        }
    }

    if(!cardNumbersToCheckForRenewals.isEmpty()){
        System.debug(cardNumbersToCheckForRenewals.keySet());

        for(Card_Request__c card : [SELECT Id, Card_Status__c, Carer_Sub_Status__c, SC_Contact__c, SC_Contact__r.Id FROM Card_Request__c WHERE SC_Contact__c IN :cardNumbersToCheckForRenewals.keySet() AND Card_Type__c = 'Carers Card']){               
            existingRenewalCardsRequests = new List<Card_Request__c>();
            for(Card_Request__c Newcard : cardRequestList){
                if(Newcard.Id != card.Id && !existingRenewalCardsRequests.contains(card)){
                    //Found a renewal card
                    card.Card_Status__c = 'Inactive';
                    existingRenewalCardsRequests.add(card);
                    continue;
                }
            }
        }
    }
    System.debug(existingRenewalCardsRequests);
    update existingRenewalCardsRequests;
}
7

Using a Map to collect the values you want to update is one useful technique:

    existingRenewalCardsRequests = new Map<Id, Card_Request__c>();
    ...
               existingRenewalCardsRequests.put(card.Id, card);
    ...
update existingRenewalCardsRequests.values();

though be careful to not just hide a logical error in the process.

It is also usually better to use a Map rather than doubly nested loops to find matching records - see e.g. Fixing a common cause of System.LimitException: Apex CPU time limit exceeded.

0

Here you are doing the mistake, for the first Card_Request__c of the first loop, you are iterating the whole list of cardRequestList and as per your condition, maximum one record does not match your criteria and others will satisfy and added to list existingRenewalCardsRequests. Now for the second record, again the process will repeat and it will add the duplicate values in your list.

for(Card_Request__c card : [SELECT Id, Card_Status__c, Carer_Sub_Status__c, SC_Contact__c, SC_Contact__r.Id FROM Card_Request__c WHERE SC_Contact__c IN :cardNumbersToCheckForRenewals.keySet() AND Card_Type__c = 'Carers Card']){               
    for(Card_Request__c Newcard : cardRequestList){
       if(Newcard.Id != card.Id){
             //Found a renewal card
             card.Card_Status__c = 'Inactive';
             existingRenewalCardsRequests.add(card);
        }

you need to check in your if condition that the value you are going to add is already in it or not. Add only if it is not in the list.

for(Card_Request__c card : [SELECT Id, Card_Status__c, Carer_Sub_Status__c, SC_Contact__c, SC_Contact__r.Id FROM Card_Request__c WHERE SC_Contact__c IN :cardNumbersToCheckForRenewals.keySet() AND Card_Type__c = 'Carers Card']){               
        for(Card_Request__c Newcard : cardRequestList){
           if(Newcard.Id != card.Id && !existingRenewalCardsRequests.contains(card)){
                 //Found a renewal card
                 card.Card_Status__c = 'Inactive';
                 existingRenewalCardsRequests.add(card);
            }

But it is not a best practice to use nested for loop otherwise you will get probably this error(System.LimitException: Apex CPU time limit exceeded) when running your code for a large number of record.

you have not given the full code snippet, so I could suggest making full use of maps to get out from these kinds of situations.

1
  • Hi Sanket, Thanks for the response but, Coming up with recursive error I have edited the question with complete code. first error: SELF_REFERENCE_FROM_TRIGGER, Object (id = a0C5D000001Riey) is currently in trigger TriggerCardRequest, therefore it cannot recursively update itself: [] – Mohammed Imran Sep 19 '18 at 23:29

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.