Best practice for function/subroutine calls

Question

Which is better? To test if a function should be called and then call it. Or call it always and let it decide whether it should do any processing?

This:

function myfunction() {
    alert("Hi");
}

if ((a || b) && c) {
   myfunction();
}

or this:

function myfunction (a, b, c) {
    if ((a || b) && c) {
       alert("Hi");
    }
}

myfunction();

I ask because I have a lot of loops and various complex if statements which then call various functions. I'm thinking I should leave the testing outside of the function to better show control flow and the very slight improved processing speed (not calling the function every time).

But on the other hand, these if tests clutter up my higher level routines and by always calling the function and letting it decide what to do seems intuitively simpler and contains logic associated with a function in that function.

This is just a best practices question. Thanks.

Answer 1

That's a very good question that probably can't be correctly answered without knowing more about the context. However, I will try to help you finding your own answer. From another answer I saw on the subject, it is said that every function should follow the single responsability principle and if that function has the responsability to know if it should execute itself and the responsbility of doing it's own work, it could be seen as if it's doing two things.

Also, if we simplify the problem enough, we end up with something like this:

function (execute) {
    if (!execute) {
        return;
    }
    //other code
}

I must say that looking at the previous code, I haven't any issues to identify that it seems like a bad design.

So far, it seems like we shouldn't put these conditions inside the function itself, but we still want to stay DRY!

What I would do is to create another function to encapsulate this logic, so that every function has a single responsability and we still dont repeat ourselves. Also note that the names makes more sense now, because we wouldn't be calling doThis while having a chance that it won't actually do this.

function doThisIfNeeded(a, b, c) {
    if ((a || b) && c) {
        doThis();
    }
}

function doThis() {
    //do something
}