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.

Improve this question

1 Answer 1

Reset to default
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
5
  • 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, 2015 at 13:21
  • Most Welcome @SamCousins .
    – Ratan Paul
    Nov 26, 2015 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, 2015 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, 2015 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, 2015 at 2:07

You must log in to answer this question.