0
public class MyPageController {
    public void recalculateTotals() {
        List<MyCustomObject__c> myCustomObjects = [SELECT Id, Totals__c, Recalculate__c, Value1__c FROM MyCustomObject__c];
        for (MyCustomObject__c obj : myCustomObjects ) {
            if (obj.Recalculate__c) {
                obj.Recalculate__c = false;
                obj.Totals__c = obj.Value1__c + obj.Value2__c;

                update obj;
            }
        }
    }
}
private class PageControllerTest {
    public void recalculateTotalsTest() {  
        //complete
    }
} 

I would like to know if you can help me with that, is for my work and they didn't give me the context, only they told me that i have th¿o check that and see if i can use another method ion order to have a better execution of the code. Kind regards

2
  • You can check for developer.salesforce.com/wiki/apex_code_best_practices
    – NITHESH K
    Commented Jul 10, 2021 at 13:53
  • 2
    This works based on what information/specifics you give us to ask a specific question, see How to Ask. Other than just giving us the code, it's important to give us the context (ex. is this for a test, challenge, what's the requirement) as well as any issues you've run into. You can edit your question to provide more info. On first look, I'd ask why you're not using a formula field on that object for this functionality vs. code/VF page. Commented Jul 10, 2021 at 14:14

2 Answers 2

2

This code is definitely not the right solution in some way or another, but it's not clear exactly what. As Kris points out in the comments, the obvious and almost certainly correct solution is a formula field: no code at all.

If for some reason a formula field is contraindicated, which seems like a stretch to me, the correct solution is almost certainly a Batch Apex class (probably scheduled). That Batch class should also include the code-level corrections offered by sfdcfox in his excellent answer.

1

You can't put a DML in a loop; you'll hit an exception if you try to update more more than 150 records this way.

You should query only records that have the checkbox checked to avoid the if statement, and reduce governor limit usage.

You didn't query Value2__c, so you can't use it in your calculations.

You don't need to query a field that you don't intend to read from. Totals__c does not need to be queried. You also never need to query Id, unless it's your only field.

This reduces your final code to:

public class MyPageController {
    public void recalculateTotals() {
        List<MyCustomObject__c> myCustomObjects = [SELECT Value1__c, Value2__c FROM MyCustomObject__c WHERE Recalculate__c = true];
        for (MyCustomObject__c obj : myCustomObjects ) {
            obj.Recalculate__c = false;
            obj.Totals__c = obj.Value1__c + obj.Value2__c;
        }
        update myCustomObjects;
    }
}

For your unit test, you just need to insert a record that matches the criteria, call your method, and then validate the output:

MyCustomObject__c testValue = new MyCustomObject__c(Recalculate__c=true, Value1__c=5, Value2__c=10);
Test.startTest();
MyPageController.recalculateTotals();
Test.stopTest();
testValue = [SELECT Totals__c FROM MyCustomObject__c];
System.assertEquals(15.0, testValue.Totals__c, 'Expected a result of 15 from recalculate method');

You must log in to answer this question.

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