Batch Class Removing all .pdf Documents Except first and most recent where name is "x"

Question

Similar to my question yesterday, the title pretty much explains what I'm trying to do. I want to remove all .pdf documents from Contact records with the exception of the first and last where the name starts with "Resume_". I have a snippet that will do that (this will be 'converted' into a Batch class) as below:

List<Attachment> attachmentList = new List<Attachment>();
List<Attachment> toDelete = new List<Attachment>();

for (Contact c : [SELECT Id FROM Contact WHERE Id = '0032500000M1uXO']) {
  for (Attachment a : [SELECT Id, Name, CreatedDate, ContentType FROM Attachment WHERE ParentId = :c.Id And ContentType = 'application/pdf' ORDER BY CreatedDate DESC]) {
    attachmentList.add(a);
  }

  for (Attachment a : attachmentList) {
    if (a.Name.startsWithIgnoreCase('Resume')) {
      toDelete.add(a);
    }
  }

  if (toDelete.size() > 2) {
    toDelete.remove(toDelete.size() - 1);
    toDelete.remove(0);

    //DELETE toDelete;
  }

  attachmentList = null;
  toDelete = null;
}

It's pretty simple:

• I'm creating two empty lists to store attachments.
• I'm looping through each contact record (in this instance, only one which is for testing)
• Whilst in that record I'm running another query that will loop through the Attachment objects to add all attachments to another list, to specifically find .pdf files
• Now I loop through this list to find files only starting with "Resume_". These are being prepped for deletion
• Finally, if the list size is greater than 2, I want to remove the first and last documents in that array. Because these should be ordered by created date, this should remove the first entry of a .pdf file and the most recent from the toDelete array, it'll then perform a delete operation
• If it isn't greater than 2, it means there's one or less .pdf documents in there and we'd want to keep those anything and do nothing
• Finally, the arrays are set to null so all data is removed, ready for the next record.

Whilst this should work, I'm concerned that I have a loop in a loop and know this is bad practise. But I'm struggling to find a more bulkified way of doing this. Because this will be executed in a Batch class, does it matter in this instance as they're processed one after another? I should stress I'm unfamiliar with Batch classes, but made my first which did what I wanted yesterday.

Answer 1

You can just change your outer loop to include a subquery for all the attachments, like so:

for (Contact c : [SELECT Id, (SELECT Id, Name, CreatedDate, ContentType FROM Attachments WHERE ContentType = 'application/pdf' ORDER BY CreatedDate DESC) FROM Contact WHERE Id = '0032500000M1uXO']) {
    for (Attachment a : c.Attachments) {
        //do logic
    }
}

However, the requirement seems like something that could easily be implemented in a trigger on Attachment. If an attachment is uploaded for a contact and it starts with "Resume", delete the previous one (except if that is the previous one is the first one). Of course, if attachments will be created in bulk, you might still need a batch, but at first glance, a batch seems unnecessary in this case.

Answer 2

for (Contact c : [SELECT Id FROM Contact WHERE Id = '0032500000M1uXO']) {
  for (Attachment a : [SELECT Id, Name, CreatedDate, ContentType FROM Attachment WHERE ParentId = :c.Id And ContentType = 'application/pdf' ORDER BY CreatedDate DESC]) {
    attachmentList.add(a);
  }

  for (Attachment a : attachmentList) {
    if (a.Name.startsWithIgnoreCase('Resume')) {
      toDelete.add(a);
    }
  }

instead use subquery and merge both for loop. You dont need to use attachmentList

for (Contact c : [SELECT Id, (SELECT Id, Name, CreatedDate, ContentType FROM Attachments WHERE ContentType = 'application/pdf' ORDER BY CreatedDate DESC) FROM Contact WHERE Id = '0032500000M1uXO']) {
    for (Attachment a : c.Attachments) {
        if (a.Name.startsWithIgnoreCase('Resume')) {
           toDelete.add(a);
        }
    }
}