0

I am having some trouble to translate my trigger to a trigger handler how do I deal with the for(Invoice__c record:Trigger.new) part?

Trigger:

trigger UpdateAutoNumber on Invoice__c (before insert) {
    // Provide default values if settings do not exist.
    InvoiceCounter__c  settings = new InvoiceCounter__c (name='Invoice__c',Amsterdam__c =0,London__c =0, Los_Angeles__c  =0, New_York__c =0, Shanghai__c =0, Tokyo__c=0);
    // Try and load the custom settings.
    for(InvoiceCounter__c  setting:[select id, name , Amsterdam__c, London__c, Los_Angeles__c, New_York__c, Shanghai__c, Tokyo__c  from InvoiceCounter__c  where name='Invoice__c'])
        settings = setting;
    // Loop through the records in the trigger.
    for(Invoice__c record:Trigger.new) {
        if(record.Department__c == 'Amsterdam') {
            record.Invoice_number__c   = 'Amsterdam-' + String.valueOf(settings.Amsterdam__c ++);
        }
        if(record.Department__c == 'London') { record.Invoice_number__c  = 'London-' + String.valueOf(settings.London__c ++);
                                             }
        if(record.Department__c == 'Los Angeles') { record.Invoice_number__c  = 'Los Angeles-' + String.valueOf(settings.Los_Angeles__c ++);
                                                  }
        if(record.Department__c == 'New York') { 
            record.Invoice_number__c  = 'New York-' + String.valueOf(settings.New_York__c ++);
        }
        if(record.Department__c == 'Shanghai') {
            record.Invoice_number__c  = 'Shanghai-' + String.valueOf(settings.Shanghai__c ++);
        }
        if(record.Department__c == 'Tokyo') {
            record.Invoice_number__c  = 'Tokyo-' + String.valueOf(settings.Tokyo__c ++);
        }
    }
    upsert settings;
}

My Trigger & Handler

trigger UpdateAutoNumber on Invoice__c (before insert) {

    UpdateAutonumberHandler handler = new UpdateAutonumberHandler();

    if(Trigger.isInsert){
        handler.OnBeforeInsert(Invoice__c record Trigger.new);
    }

}



public class UpdateAutonumberHandler {

    public static void OnBeforeInsert(List<Invoice__c> record){
     // Provide default values if settings do not exist.
    InvoiceCounter__c  settings = new InvoiceCounter__c (name='Invoice__c',Amsterdam__c =0,London__c =0, Los_Angeles__c  =0, New_York__c =0, Shanghai__c =0, Tokyo__c=0);
    // Try and load the custom settings.
    for(InvoiceCounter__c  setting:[select id, name , Amsterdam__c, London__c, Los_Angeles__c, New_York__c, Shanghai__c, Tokyo__c  from InvoiceCounter__c  where name='Invoice__c'])
        settings = setting;
    // Loop through the records in the trigger.

        if(record.Department__c == 'Amsterdam') {
            record.Invoice_number__c   = 'Amsterdam-' + String.valueOf(settings.Amsterdam__c ++);
        }
        if(record.Department__c == 'London') { record.Invoice_number__c  = 'London-' + String.valueOf(settings.London__c ++);
                                             }
        if(record.Department__c == 'Los Angeles') { record.Invoice_number__c  = 'Los Angeles-' + String.valueOf(settings.Los_Angeles__c ++);
                                                  }
        if(record.Department__c == 'New York') { 
            record.Invoice_number__c  = 'New York-' + String.valueOf(settings.New_York__c ++);
        }
        if(record.Department__c == 'Shanghai') {
            record.Invoice_number__c  = 'Shanghai-' + String.valueOf(settings.Shanghai__c ++);
        }
        if(record.Department__c == 'Tokyo') {
            record.Invoice_number__c  = 'Tokyo-' + String.valueOf(settings.Tokyo__c ++);
        }

    upsert settings;


}
| improve this question | | | | |
  • 3
    This code has a fundamental problem: if two transactions happen closely enough, you'll hit a race condition. Consider adding FOR UPDATE to your query. Also, for what it's worth, using dynamic apex to determine the field name could significantly reduce the code size. – sfdcfox Jul 25 '18 at 4:28
  • What is your question? – Adrian Larson Jul 25 '18 at 16:58
  • @sfdcfox Good spot! I never actually read what the handler was doing :) – Matt Lacey Jul 27 '18 at 0:39
3

You don't need to specify the type or name of parameters when calling methods in Apex.

This:

handler.OnBeforeInsert(Invoice__c record Trigger.new);

Should be:

handler.OnBeforeInsert(Trigger.new);

Trigger.new is a list of Invoice__c records, i.e. it is of the type List<Invoice__c> which is what your method expects for the record parameter:

public static void OnBeforeInsert(List<Invoice__c> record){

which, while we're at it, should probably be called records since it's a collection, not a singular Invoice__c.

| improve this answer | | | | |
  • Thanks! The static part also needed to be removed. – Thomas Jul 25 '18 at 4:45
  • I'd not noticed you'd make it static. It actually makes more sense to have it static, it just means when you call it you don't create a new instance of TriggerHandler, you just call TriggerHandler.OnBeforeInsert(Trigger.new); – Matt Lacey Jul 25 '18 at 4:49

Not the answer you're looking for? Browse other questions tagged or ask your own question.