Apex trigger - is this the best approach?

Question

I'm getting up to speed with Apex - I have written a trigger that works, but is this the approach that an experienced coder would take?

I have a custom object (vist_detail__c) which has a lookup to Contracts. We create tasks on the custom object. I want an alert when the task date is beyond the Contract end date.

(The commenting shows that I began by adding the trigger.new IDs to a set but then I couldn't figure out how to access the Task date to compare with Contract end date so I dropped the set idea)

trigger MaCwTaskTriggerTest on Task (after insert, after update) {
    system.debug('Hey Dude');
//    Set<Id> cwIds = new Set<Id>();
    for (Task aTask: Trigger.new)
    {
//    cwIds.add(aTask.WhatId);
//      system.debug('Added to set: '+aTask.WhatId);
//  }
//    for (Id i : cwIds) {
//        system.debug('Id in list is '+i);

        vist_Detail__c vD = [SELECT contract__r.endDate FROM vist_Detail__c 
                                    WHERE Id = :aTask.WhatId];
          Date cEndDate = vD.contract__r.endDate;
          system.debug('Contract end date: '+cEndDate);
          if (aTask.ActivityDate > cEndDate) {
            system.debug('Task Date greater than Contract end date');                                  
            }
          else {
            system.debug('Task Date less than or equal to Contract end date');                                  
               }
    }
}

It works, but is it best practice?

Any suggestions on how best to alert a user? The task is created by workflow. I can't see a situation where these tasks would be created in bulk in which case would emailing user be sensible? I have the Apex for that.

Many Thanks, Mike.

Answer 1

Problem 1:

Query inside for loop

Problem 2:

Without checking the whatId of Task to be Id of Vist_Detail__c, the query can return 0 records and throw

List has no rows for assignment error

Look for documentation to rectify the above mentioned errors.

Hope it helps. Happy learning!

Answer 2

Good catch - Thanks @vigneshwaran.

Like this?

trigger MaCwTaskTriggerTest on Task (after insert, after update) {
    Set<Id> tIds = new Set<Id>(); // Task IDs
    Set<Id> cwIds = new Set<Id>(); // Task WhatIDs
    Map<Id, Task> TMap;
    Map<Id, vist_Detail__c> vDMap;


    for (Task aTask: Trigger.new)
    {
      cwIds.add(aTask.WhatId);
      system.debug('Task.WhatId added to set: '+aTask.WhatId);
      tIds.add(aTask.Id);
      system.debug('Task.Id added to set: '+aTask.Id);
    }

    TMap = new Map<Id, Task>([SELECT Id, ActivityDate FROM Task 
                                    WHERE Id in :tIds]);
    vDMap = new Map<Id, vist_Detail__c>([SELECT Id, contract__r.endDate FROM vist_Detail__c 
                                    WHERE Id in :cwIds]);
    system.debug('vDMap = '+vDMap);

        if (vDMap.size() > 0) {
            for (Task aTask: Trigger.new) {
              system.debug ('Task Activity Date: '+TMap.get(aTask.Id).ActivityDate);
              system.debug ('Contract End Date: '+vDMap.get(aTask.WhatId).contract__r.endDate);
                if (TMap.get(aTask.Id).ActivityDate > vDMap.get(aTask.WhatId).contract__r.endDate)
                system.debug('Task Date greater than Contract end date');                                  
            else
                system.debug('Task Date less than or equal to Contract end date');                                  
            }
        }
       else system.debug('No VDs selected');
    }