is there a way to optimized this code? I am having problem with bulk upload and these needs to be optimized, can you instruct me on what should be the proper way? I'm getting System.LimitException: Apex CPU time limit exceeded

public static void populateLastestEnrollment(List<Case> pCase){
    Id EnrollmentRecordType = Schema.Sobjecttype.Case.getRecordTypeInfosByName().get('Enrollment').getRecordTypeId();
    Id DiscontinuedmentRecordType = Schema.Sobjecttype.Case.getRecordTypeInfosByName().get('Unenroll From GPS').getRecordTypeId();
    Id AdultPatient = Schema.SObjectType.Account.getRecordTypeInfosByName().get('Adult Patient').getRecordTypeId();
    Id MinorPatient = Schema.SObjectType.Account.getRecordTypeInfosByName().get('Minor Patient').getRecordTypeId();
    Id RECORDTYPEID_RELATIONSHIP_INDIVIDUAL = Schema.SObjectType.Relationship__c.getRecordTypeInfosByName().get('Individual Relationship').getRecordTypeId();

    List<Account> updatedAccountList = new List<Account>();
    Set<Id> caseAccountId = new Set<Id>();

    //Get the parent Account of the case
    for(Case cs : pCase){
        caseAccountId.add(cs.AccountId);
    }

    List<Account> caseAccountList = [SELECT Id, Latest_Enrollment_Unenrollment_Case__c, Consent_Provided__c,Consent_Version__c, Consent_Provided_Date__c, 
                                     Enrollment_Status_Text__c, Enrollment_Form_Received_Date__c, RecordTypeId, X18th_Birthdate__c, PersonMobilePhone, PersonHomePhone,
                                     (SELECT Id, GPS_Enrollment_Status_Detail__c, Date_Consent_Provided__c, GPS_Enrollment_Status__c, Enrollment_Type__c,
                                      Enrollment_Form_Received_Date__c, Consent_Version__c, CaseNumber, Consent_Provided__c, Consent_Provided_By__c,
                                      CreatedDate, Status, RecordTypeId, Case_Sub_Status__c FROM Cases ORDER BY CreatedDate ASC) 
                                     FROM Account WHERE Id IN: caseAccountId];

    //loop through all child records
    for(Account a : caseAccountList){
        //Checks if case object has records
        if(a.Cases.size() > 0){
            // find out which one is the most recent relevant case for a given patient account
            for(Case c : a.Cases){
                if(c.CreatedDate >= mostRecentCase.CreatedDate || mostRecentCase.Id == null){
                    if(c.RecordTypeId == EnrollmentRecordType || 
                       (c.RecordTypeId == DiscontinuedmentRecordType
                        && c.Status == 'Closed'
                        && c.Case_Sub_Status__c == 'Completed')){
                            mostRecentCase = c;
                        }                     
                }
            }
        }

        // If there is no relevant case available, then make the auto populated fields null:
        if(mostRecentCase.Id == null){
            a.Latest_Enrollment_Unenrollment_Case__c = null;
            a.Consent_Provided_Date__c = null;
            a.Consent_Expiration_Date__c = null;
            a.Consent_Provided__c = null;
            a.Consent_Version__c = null;
            a.Enrollment_Form_Received_Date__c = null;
            a.Enrollment_Status_Text__c = 'Never Enrolled';
        } else if(mostRecentCase.RecordTypeId == EnrollmentRecordType &&
                  (mostRecentCase.Enrollment_Type__c != 'New Consent' || mostRecentCase.Date_Consent_Provided__c != null)){
            a.Consent_Provided__c = mostRecentCase.Consent_Provided__c;
            a.Consent_Provided_Date__c = mostRecentCase.Date_Consent_Provided__c;
            a.Consent_Version__c = mostRecentCase.Consent_Version__c;
            if (a.Consent_Provided__c == null){
                a.Consent_Expiration_Date__c = null;
            } else if (a.Consent_Provided__c == 'Verbal Consent') {
                a.Consent_Expiration_Date__c = a.Consent_Provided_Date__c.addDays(30);
            } else if (a.Consent_Provided__c == 'Written Consent') {
                if (a.X18th_Birthdate__c.addDays(30) < a.Consent_Provided_Date__c.addYears(10) && a.X18th_Birthdate__c > mostRecentCase.Date_Consent_Provided__c){ // Sheri updated add days
                    a.Consent_Expiration_Date__c = a.X18th_Birthdate__c.addDays(30); 
                } else {
                    a.Consent_Expiration_Date__c = a.Consent_Provided_Date__c.addYears(10);
                }
            }
            a.Consent_Expiration_Workflow_Reset__c = false;
            a.Enrollment_Status_Text__c = mostRecentCase.GPS_Enrollment_Status__c;
            a.Enrollment_Form_Received_Date__c = mostRecentCase.Enrollment_Form_Received_Date__c;       
            a.Latest_Enrollment_Unenrollment_Case__c = mostRecentCase.Id;
        } else if(mostRecentCase.RecordTypeId == DiscontinuedmentRecordType){
            a.Consent_Provided__c = null;
            a.Consent_Provided_Date__c = null;
            a.Consent_Expiration_Date__c = null;
            a.Consent_Version__c = null;
            a.Enrollment_Status_Text__c = mostRecentCase.GPS_Enrollment_Status__c;
            a.Latest_Enrollment_Unenrollment_Case__c = mostRecentCase.Id;
        }
        //Make sure that only 1 record will update the Parent
        updatedAccountList.add(a);
    }

    //update Account
    if(updatedAccountList.size() > 0){
            update updatedAccountList;
    }

}
share|improve this question
3  
In general, you should show some effort to do some research and understand the problem. This question may stay open if you're lucky, but questions which do not demonstrate an attempt to solve the problem can be harshly received here. With this much code, it's really broad and there are likely to be scores of optimizations that can be made. – Adrian Larson 14 hours ago
    
Try to use maps instead of nested for loops and attempt your first optimization – Santanu Boral 14 hours ago
    
@SantanuBoral Nested loops are okay here; they're iterating over each case per account, not all queried cases per account. – sfdcfox 14 hours ago
    
Also where is mostRecentCase even coming from? You don't seem to assign it within this method body. – Adrian Larson 13 hours ago

There are a number of optimizations that could occur here, but none of them are "major." Here goes.

Stop Describing!

This one may or may not be your main culprit, because you're not caching your describe results, potentially wasting hundreds of milliseconds in CPU time or more, depending on the quantity of record types you have.

Map<String, RecordTypeInfo>
  crti = Case.SObjectType.getDescribe().getRecordTypeInfosByName(),
  arti = Account.SObjectType.getDescribe().getRecordTypeInfosByName();
Id 
  EnrollmentRecordType = crti.get('Enrollment').getRecordTypeId(),
  DiscontinuedmentRecordType = crti.get('Unenroll From GPS').getRecordTypeId(),
  AdultPatient = arti.get('Adult Patient').getRecordTypeId(),
  MinorPatient = arti.get('Minor Patient').getRecordTypeId(),
  RECORDTYPEID_RELATIONSHIP_INDIVIDUAL = Relationship__c.SObjectType.getRecordTypeInfosByName().get('Individual Relationship').getRecordTypeId();

Don't Ask to Loop

Things like if(a.Cases.size() > 0) and if(updatedAccountList.size() > 0) are not necessary, although this optimization is mostly a matter of code size difference, not so much CPU time, it's simply a lot easier to read if you don't do this.

Reduce Statement Count

When we were in the old days of 200,000 script lines per execution context, we had to cram together multiple operations. While we no longer have to do this, it is often convenient to cram together similar operations as a matter of performance.

To that end, using a new SObject constructor is at least twice as fast in CPU time than using separate statements. This means you would do well to do something like this:

Account tempAccount;
if(mostRecentCase.Id == null) {
    tempAccount = new Account(Id=a.Id, Latest_Enrollment_Unenrollment_Case__c=null,
        Consent_Provided_Date__c=null,
        Consent_Expiration_Date__c=null,
        Consent_Provided__c=null,
        Consent_Version__c=null,
        Enrollment_Form_Received_Date__c=null,
        Enrollment_Status_Text__c='Never Enrolled');
} else if(...) {
} ...
if(tempAccount != null) {
    updatedAccountList.add(tempAccount);
}

I recognize that it is not always possible to do this, but you should always try to move as many assignments as possible inside an SObject constructor, as you will shave precious time off your CPU usage.

Also notice that I use this technique in the first section. Execution governor limits are only checked occasionally when a semi-colon is encountered, so using the comma operator reduces the number of governor limit checks that will occur.

Filter over Condition

Time spent in a query does not count towards CPU limits, but time spent in an if statement does. Since you're only interested in two record types, you can filter for them in your query. As a further optimization, you can use ORDER BY CreatedDate DESC LIMIT 1 to find the most recent matching case without a for loop at all.

List<Account> caseAccountList = 
  [SELECT Latest_Enrollment_Unenrollment_Case__c, Consent_Provided__c,Consent_Version__c, Consent_Provided_Date__c, 
          Enrollment_Status_Text__c, Enrollment_Form_Received_Date__c,
          RecordTypeId, X18th_Birthdate__c, PersonMobilePhone, PersonHomePhone,
          (SELECT GPS_Enrollment_Status_Detail__c, Date_Consent_Provided__c, GPS_Enrollment_Status__c, Enrollment_Type__c,
                  Enrollment_Form_Received_Date__c, Consent_Version__c, CaseNumber, Consent_Provided__c, Consent_Provided_By__c,
                  CreatedDate, RecordTypeId FROM Cases
           WHERE RecordType.Name IN ('Enrollment','Unenroll from GPS') AND Status = 'Closed' AND Case_Sub_Status__c = 'Completed' 
           ORDER BY CreatedDate DESC LIMIT 1) 
   FROM Account WHERE Id IN: caseAccountId];

Given what I've said here, I doubt these optimizations alone will solve your problem. You're going to need to analyze any account triggers, processes, and so on to determine what the problem is. It's more likely a systemic overuse of CPU-intensive statements, such as constant describes when not necessary that are causing the actual performance problem rather than a single "this is the line of code that broke."

share|improve this answer

The first step when you have a performance problem is to measure to find out which part of code is causing the problem; one loop is usually the culprit. You can do that by adding System.debug calls at each significant point in the code. You may even discover that the problem is in a different piece of code e.g. a trigger that runs when your update the Accounts.

(Serious) performance problems usually relate more to algorithms than individual lines of code. A common candidate problem area is doubly nested loops where each loop is iterating over a large collection because performance there degrades exponentially (i.e. quicker than you might expect).

If the problem turns out to be in the for(Case c : a.Cases) loop, these terms:

                if(c.RecordTypeId == EnrollmentRecordType || 
                   (c.RecordTypeId == DiscontinuedmentRecordType
                    && c.Status == 'Closed'
                    && c.Case_Sub_Status__c == 'Completed')){

look like they could all be moved to the WHERE clause of the FROM Cases part of the query. Potentially you could avoid an Apex loop altogether with something like this:

FROM Cases WHERE ... ORDER BY CreatedDate DESC LIMIT 1

The work still has to be done, but the database is a more efficient place to do that work.

One other point is that you might find writing down exactly what you need the code to achieve and then thinking of alternative ways to accomplish that more productive than trying to rework your current code.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.