Help with for loop matching in batch class

Question

I'm having an issue trying to design a for loop in a batch process for deleting apex share records. If anyone could help it'd be much appreciated.

The basic scenario is as follows. First, there is a master-detail between objects Parent and Child. When a child is created, it's Status__c picklist field is set to 'Active' and a Parent__Share record is created for it (in another trigger). The UserOrGroupId on the Parent__Share record is equivalent to a user lookup Team_Member__c field on the child object (thus giving that user access via the Parent__Share).

The class below should be called when the Status__c picklist field is changed to 'Inactive'. In that case, any Parent__Share record where the UserOrGroupId equals Team_Member__c on a Child record and where the Child has a Status of 'Inactive', and where they share the same Parent should be deleted..

Two questions: 1) Am I going about this in right way? Is using embedded for loops the correct approach? 2) What is wrong with my code?

Thank you to anyone who can help :-)

global class DeleteShares implements Database.Batchable<sObject> {

global Database.QueryLocator start(Database.BatchableContext BC){
    return Database.getQueryLocator([SELECT Id FROM Parent__c]);  
}

global void execute(Database.BatchableContext BC, List<sObject> scope){

    // Create map for Parent object
    Map<Id, Parent__c> parentMap = new Map<Id, Parent__c>((List<Parent__c>)scope);

    //Get all existing share records       
    List<Parent__Share> currentShares = [SELECT Id, UserOrGroupId FROM Parent__Share WHERE ParentId IN 
        :parentMap.keySet()
        AND RowCause = 'Team_Member_Access__c'];

    // Create list for child object
    List<Child__c> childList = new List<Child__c>();
    childList = [SELECT Id, Parent__c, Status__c, Team_Member__c FROM Child__c WHERE Status__c = 'Inactive'];

    //When a child record is created, a trigger fires that creates a share record (Parent__Share)
    //When created, the child record is also given a Status of 'Active' in a picklist field Status__c on the child record

    // My matching loop. 
    // When the Status on the child record is changed from 'Active' to 'Inactive',
    // this class will be called. All share records with RowCause= 'Team_Member_Access__c' 
    // and a child record that has Status 'Inactive' should be deleted
    for (Parent__Share s: currentShares){
      for (Child__c c: childList){
          if (parentMap.containsKey(c.Parent__c) && (s.UserOrGroupId == c.Team_Member__c)){

              Delete s;
              }
             }
        }

    } 

    global void finish(Database.BatchableContext BC){}
}

.

Answer 1

The first thing I see wrong with the code is that you have a DML operation inside a for loop and should be changed to:

for (Parent__Share s: currentShares){
      for (Child__c c: childList){
          if (parentMap.containsKey(c.Parent__c) && (s.UserOrGroupId == c.Team_Member__c)){
               shareRecordsToDelete.add(s);
          }
      }
 }
 delete shareRecordsToDelete;

In the code you need to declare a list that will hold share records, List<Parent__Share> shareRecordsToDelete = new List<Parent__Share>();

This will then remove the DML operation outside the for loop, preventing you to hit the limit.

Can I also ask, could this SOQL query be limited in terms of records returned SELECT Id FROM Parent__c ? It might be that you don't need all the records in the Parent__c object and it will avoid in the future hitting the limit of 50.000 records return from the database.