How can I optimize the code below?

Question

Problem statement:

I need to prevent the user from creating more than 10 Opportunities per month.

public static void createOpportunity(List<Opportunity> newOpportunityList){

        for(Opportunity newOpportunity : newOpportunityList){
        
            for(AggregateResult result : [SELECT COUNT(CreatedById) FROM Opportunity WHERE createdByID=: System.UserInfo.getUserId() AND createdDate = THIS_Month]){
                Integer count = (Integer)result.get('expr0');
                if(count > Integer.valueOf(System.Label.OpportunityThreshold)){
                    newOpportunity.addError('This user has had more than 10 opportunity in this month')  ;      
                } 
            }
        }
    }

Primarily, I want to learn if there's a better way to use loops here.

Answer 1

Some points:

• the code performs a query in a loop. This is an anti-pattern and will fail with a governor limit if the outer loop is larger than 100 records long.
• the query does not depend on the data in the outer loop, so does not need to be in the loop.
• the query should only return one aggregate result. Using a loop is still easiest for processing the results.

With this in mind, try:

public static void validateNewOpportunities(List<Opportunity> newOpportunities) {
    for (AggregateResult result : [SELECT COUNT(CreatedById) FROM Opportunity WHERE createdByID = :UserInfo.getUserId() AND createdDate = THIS_MONTH]) {
        Integer count = (Integer) result.get('expr0');

        if (count > 10) {
            for (Opportunity newOpportunity : newOpportunities) {
                newOpportunity.addError(Label.TooManyOpportunitiesByUserInMonth);
            }
        }
    }
}

You no longer have a query in a loop, which is the primary concern here.

By the way, I have hard coded the threshold here - use of a label is simply wrong, lazy coding and an abuse of labels; to make it configurable use a custom metadata type or custom settings. I have, however, shown use of a label for the error message which is entirely appropriate.