Schedulable batch class to insert a list then update a related parent list

Question

I am trying to create a scheduled batch class to query existing records for a custom object, add it to a list then insert the list of records for the same custom object, then update a custom field for a list belonging to the parent object, via a lookup field . The GF__c is the child object, and the Opportunity is the parent object. when I run it, it does create the new list of child records, but I am getting an error regarding the parent list:

Duplicate id in List::...

and the custom field of the parent isn't getting updated. I've posted my code below, any assistance is appreciated.

public class GF__cBatch implements Database.Batchable<SObject>, Schedulable {
    public GF__cBatch() {}
    public void execute(SchedulableContext sc) {
        Database.executeBatch(this, 60);
    }
    public Database.QueryLocator start(Database.BatchableContext bc) {
        return Database.getQueryLocator([SELECT Id, Opportunity_Name__c, Forecast_Month_Year_Current_Month_Year__c FROM GF__c WHERE Forecast_Month_Year_Current_Month_Year__c=true]);
    }
    public void execute(Database.BatchableContext bc, List<sObject> scope) {
        list<GF__c> gfList = new list<GF__c>();
        list<Opportunity> OppList = new list<Opportunity>();
        datetime myDateTime = datetime.now();        
        For(Sobject s :scope){
            GF__c GF = (GF__c) s;           
            gfList.add(new GF__c(                
                Opportunity_Name__c = GF.Opportunity_Name__c                                               
            ));
            Opplist = [SELECT Id, Forecast_Month_Year__c FROM Opportunity WHERE Id = :GF.Opportunity_Name__c];            
            OppList.add(new Opportunity(
                Id = GF.Opportunity_Name__c,                
                Forecast_Month_Year__c = myDateTime.format('MMMM, YYYY')
            ));   
        }        
        try{
            insert gfList;            
            if(OppList.size() > 0){          
                update OppList;
            }
        }
        catch(Exception E){
            system.debug('Error is: ' + E.getMessage());
        }                
    }    
    public void finish(Database.BatchableContext bc) {
        system.debug('FORECAST CREATE JOB IS FINISHED');
    }
}

Answer 1

You're going to run into trouble here when you're processing more than one GF__c record with the same Opportunity as parent. The reason that's the case is that your code accumulates a List<Opportunity> to update, with one entry for each GF__c.

As a result, if you have N GF__c records with the same Opportunity, you'll have N Opportunities in the update list with the same Id. Salesforce doesn't allow that in a single DML operation.

The typical idiom is to use a Map<Id, Opportunity>, where you set each Opportunity needing to be updated in the map under its Id:

// We found an Opportunity with Id myId needing to be updated
myMap.put(myId, new Opportunity(Id = myId, ANOTHER_FIELD = VALUE, ...)

and then do

update myMap.values();

This ensures they're deduplicated appropriately, because Maps are inherently one-to-one mappings. Your code can also check whether it's already looked at a child record for that Opportunity by simply looking at whether myMap.containsKey(someOpportunityId). If, for example, you need to populate the largest value of a field on the child object, you could then pull the Opportunity instance back out of the Map and run some logic to decide which child object value you wanted to populate.

Additionally, your batch has a bulkification issue, which I assume is why you set the scope size to 60. You need to move the Opportunity query outside the loop. However, if you adopt the Map-based pattern above, it looks to me like you don't need to query at all, since you're constructing new sObject instances.