1

I am curious on if this design is considered good or bad practice.

I have a dispatcher and a service class.

Dispatcher I have the following:

public void doBeforeUpdate(Map<Id,sObject> oldMap, Map<Id,sObject> newMap, List<sObject> triggerOld, List<sObject> triggerNew) {

    Boolean myBool = service.myCheck(triggerOld, triggerNew);

    if(!myBool){
        Boolean newBool = false;

        for(sObject oldInfo: triggerOld){
            for(sObject newInfo: triggerNew){
                if(newInfo.price < oldInfo.price){
                    newBool = true;
                }
            }
        }

        if(newBool){
            service.priceChanged(triggerNew);
        }
    }

}

In my service I have the following:

public static Boolean myCheck(List<sObject> triggerOld, List<sObject> triggerNew){

    Boolean myBool = false;

    for(sObject oldInfo: triggerOld){
        for(sObject newInfo: triggerNew){
            if(newInfo.Id == oldInfo.Id && newInfo.Name != oldInfo.Name){
                myBool = true;
                break;
            }
        }
        if(myBool) break;
    }

    return myBool;

}

My question is, is it good or bad practice to have myBool in the dispatcher be set to a return method from the service to determine if the code and method in the if statement run in the dispatcher. I am working in Apex. My thought is, doing it this way saves me processing time if the first condition fails. Should I not worry about that? Is it good or bad to set a boolean based off of a return method? Is this just over kill and I am over thinking this? Any guidance would be greatly appreciated. Thank you.

  • 1
    I think you are conflating two questions. One is whether capturing a call result in a variable is ok (it is). And the other is a domain question about traversing lists, and saving processing time, which I fail to understand as it is unclear. – Erik Eidt Sep 15 '17 at 19:07
1

If you use "early return," you can avoid the Boolean variable and break statements entirely.

public static Boolean myCheck(List<sObject> triggerOld, List<sObject> triggerNew){

    for(sObject oldInfo: triggerOld){
        for(sObject newInfo: triggerNew){
            if(newInfo.Id == oldInfo.Id && newInfo.Name != oldInfo.Name){
                return true;
            }
        }
    }

    return false;
}

See Also
Computer Programming/Coding Style/Minimize Nesting/Early Return

  • Awesome thank you for that insight. That did not even cross my mind. I suppose I was not the clearest in my question. My question is more in regards to the dispatcher code that I have. I am very new to development and am unsure if having that code block set up that way is considered good or bad practice. I like the early return though! :) – bemon Sep 15 '17 at 18:00
  • If you have a better alternative available to you, you can stop thinking about whether your original alternative is good or bad. – Robert Harvey Sep 15 '17 at 18:01
0

In addition to what Robert Harvey said in their answer, you're also doing more work than necessary in your Dispatcher loop. It would be simpler to do something like this:

public void doBeforeUpdate(Map<Id,sObject> oldMap, Map<Id,sObject> newMap, List<sObject> triggerOld, List<sObject> triggerNew) {

    Boolean myBool = service.myCheck(triggerOld, triggerNew);

    if(!myBool){
        for(sObject oldInfo: triggerOld){
            for(sObject newInfo: triggerNew){
                if(newInfo.price < oldInfo.price){
                    service.priceChanged(triggerNew);
                    return;
                }
            }
        }
    }
}

This isn't just an early return. It's also (potentially) doing fewer iterations of the loop. As you had it before, you were continuing to loop after you already knew that you needed to do the price change.

There may be other ways to architect this. Rather than having to search the list to see if there are any changes, would it be possible for changes to trigger something that tells your dispatcher it will definitely need to do something? For example, when the price info changes send a notification saying an update is needed. Depending on the rest of your architecture and the nature of your data, that may or may not be the best way to handle it, but I just wanted to mention that there are other ways to do the same work, since that seems to be your question.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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