1

I have the following code:

Public Pagereference quickSave(){

    for(Rights_Tracker__c RT : RightsTrackerList)
    {

        if(RT.Actual__c > RT.Agreement__c) 
        {  
            ApexPages.addMessage(new ApexPages.Message(ApexPages.Severity.ERROR, 'Actual Bigger than Agreement'));
            return null;
        }
        else
        {

            PageReference pageRef = Page.rightsTrackerEdit;
            pageRef.getParameters().put('account', accountid);
            return pageref;
        }
    }
}

I'm trying to display an error message when RT.Actual__c is greater than RT.Agreement__c.

When the error is displayed, I'm trying to return null to refresh the page. If there's no error, I want to upsert the list and return the new pageRef.

However I'm getting the following error:

Error: Class Compile Error: Non-void method might not return a value or might have statement after a return statement. at line 167 column 9

Line 167 is the for(Rights_Tracker__c RT : RightsTrackerList) line, so I guess it can't detect my return statements in the conditionals. Am I doing something wrong or do I have to do it another way?

Thanks.

3
Public Pagereference quickSave(){
    Boolean first = false;
    for(Rights_Tracker__c RT : RightsTrackerList)
    {

        if(RT.Actual__c > RT.Agreement__c) 
        {  
            first = true;
            break;
        }
        else
        {
            upsert RightsTrackerList;
            first = false;
            break;

        }
    }
    if(first)
    {
        ApexPages.addMessage(new ApexPages.Message(ApexPages.Severity.ERROR, 'Actual Bigger than Agreement'));
        return null;
     }
    else
       {
           PageReference pageRef = Page.rightsTrackerEdit;
            pageRef.getParameters().put('account', accountid);
            return pageref;
       } 

}   

Use break statement here.

Just for a note don't perform DML operation inside for loop. You may hit governor limits

| improve this answer | |
  • Crappy proofreading on my part! It would really suck to be upserting considering the amount of items in my list. Thanks for your answer, worked perfectly! – user24634 Nov 26 '15 at 13:21
  • Most Welcome @SamCousins . – Ratan Paul Nov 26 '15 at 13:24
  • Perhaps I'm missing something, but what is the point of the for loop in this context? It seems like it's just adding unnecessary overhead when all you want is RightsTrackerList[0]. Do you need to verify that RightsTrackerList isn't null (by the way, this is the condition under which your method in the OP would leave scope without a return statement)? – IllusiveBrian Nov 26 '15 at 17:38
  • @Namfuak It's looping through the list and setting the bool to true if the condition is true. I don't think the break should be in the same part as the pagemessage statement. – user24634 Nov 27 '15 at 22:51
  • @SamCousins In your original code as well as in this answer, it will return/break regardless of the result of the if statement, so it will only iterate once at most. – IllusiveBrian Nov 28 '15 at 2:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy