We can make this code "generic", allowing your code to work on all three types at once. Observe:
public static void processRecords(SObject[] records) {
Set<Id> parentIds = new Set<Id>(), protectedIds = new Set<Id>();
for(SObject record: records) {
parentIds.add((Id)record.get('ParentId'));
}
for(ProcessInstance record: [SELECT TargetObjectId
FROM ProcessInstance where Status = 'Approved' AND TargetObjectId IN :parentIds]) {
protectedIds.add(record.TargetObjectId);
SObject[] violators = new SObject[0];
for(SObject record: records) {
if(protectedIds.contains((Id)record.get('ParentId'))) {
violators.add(record);
}
}
attachmentKillSwitch(violators);
}
public static void attachmentKillSwitch (SObject[] violators) {
User_Controls__c CS = User_Controls__c.getInstance();
FOR (SObject violator: violators){
IF(!CS.Attachments_Allow_Delete__c){
violator.addError('bogus - deleting attachments on approved records is not allowed');
}
}
}
Note that you could also avoid the IF statement within the loop-- you only need to call it at most once:
public static void attachmentKillSwitch (SObject[] violators) {
User_Controls__c CS = User_Controls__c.getInstance();
if(cs != null && cs.attachments_allow_delete__c) {
FOR (SObject violator: violators){
violator.addError('bogus - deleting attachments on approved records is not allowed');
}
}
}
Of course, as a further optimization, we're only showing the error if the user isn't allowed to delete records, so we can skip everything if they are:
public static void processRecords(SObject[] records) {
{
User_Controls__c cs = User_Controls__c.getInstance();
if(cs != null && cs.Attachments_Allow_Delete__c) {
return; // No need to go on
}
}
Set<Id> parentIds = new Set<Id>(), protectedIds = new Set<Id>();
for(SObject record: records) {
parentIds.add((Id)record.get('ParentId'));
}
for(ProcessInstance record: [SELECT TargetObjectId
FROM ProcessInstance where Status = 'Approved' AND TargetObjectId IN :parentIds]) {
protectedIds.add(record.TargetObjectId);
}
SObject[] violators = new SObject[0];
for(SObject record: records) {
if(protectedIds.contains((Id)record.get('ParentId'))) {
violators.add(record);
}
}
attachmentKillSwitch(violators);
}
public static void attachmentKillSwitch (SObject[] violators) {
FOR (SObject violator: violators){
violator.addError('bogus - deleting attachments on approved records is not allowed');
}
}
As you can tell, we've slimmed down the function to the point where the extra static call isn't necessary:
public static void processRecords(SObject[] records) {
{
User_Controls__c cs = User_Controls__c.getInstance();
if(cs != null && cs.Attachments_Allow_Delete__c) {
return; // No need to go on
}
}
Set<Id> parentIds = new Set<Id>(), protectedIds = new Set<Id>();
for(SObject record: records) {
parentIds.add((Id)record.get('ParentId'));
}
for(ProcessInstance record: [SELECT TargetObjectId
FROM ProcessInstance where Status = 'Approved' AND TargetObjectId IN :parentIds]) {
protectedIds.add(record.TargetObjectId);
}
for(SObject record: records) {
if(protectedIds.contains((Id)record.get('ParentId'))) {
violator.addError('bogus - deleting attachments on approved records is not allowed');
}
}
}
We can go even further, targeting objects directly:
public static void processRecords(SObject[] records) {
{
User_Controls__c cs = User_Controls__c.getInstance();
if(cs != null && cs.Attachments_Allow_Delete__c) {
return; // No need to go on
}
}
Map<Id, SObject[]> recordsByParent = new Map<Id, SObject[]>();
for(SObject record: records) {
Id parentId = (Id)record.get('ParentId');
if(recordsByParent.containsKey(parentId)) {
recordsByParent.get(parentId).add(record);
} else {
recordsByParent.put(parentId, new SObject[] { record });
}
}
for(ProcessInstance record: [SELECT TargetObjectId
FROM ProcessInstance where Status = 'Approved' AND TargetObjectId IN :recordsByParent.keySet()]) {
for(SObject violator: recordsByParent.get(record.TargetObjectId)) {
violator.addError('bogus - deleting attachments on approved records is not allowed');
}
}
}
At this point, I could start making even more obscure optimizations, but this answer was primarily meant to show you some techniques you can use to optimize later code.
However, all this still doesn't answer the original question. So, here's the answer you're after in a generic (no pun intended) sense: Constructors are only for initialization of objects. Yes, you can put anything (well, almost anything) you want in a constructor, but as a general rule, you should not put logic like this in a constructor. It's not that it wouldn't work, it's just not a particularly "OOP" design model.
Constructors shouldn't do anything to modify the state of the system external to itself. That's not what constructors are for. Adding errors inside constructors would violate this principle. Furthermore, since these are basically utility methods, there's really no need to have a constructor at all-- notice that your code uses static methods, which require no instance to be constructed.
As you continue your path down being an Apex Code developer, you'll notice that generics are your friend: You can use Object in place of any data type, and even convert between types at runtime, and you can use SObject to represent any type of record, even custom settings. There are some limitations to generics, of course, such as how SObject.addError doesn't support a field parameter (so, there's no way to render an error on a field generically), but most of the time, you'll find that using generics can greatly reduce the amount of code you have to write and maintain.