CODEWITHSUNDEEP
functionsubroutine
C G
C G

Reputation: 25

Best practice for function/subroutine calls

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.

Upvotes: 1

Views: 274

Answers (3)

plalx
plalx

Reputation: 43728

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
}

Upvotes: 3

sashkello
sashkello

Reputation: 17871

If that if statement is same thing always, then why duplicating it if you can put it within the function? No reason for that - this is the purpose of creating a function, to save you from retyping it multiple times... And it eliminates chances of forgetting/mistyping things. Also, if you need to change the condition or add another option, you don't need to change it everywhere, only in the function body. This makes code much easier to edit and maintain.

On a second thought, it would be even better to have a separate "alert" function which just does your alert without any extra program-specific things (which makes it reusable). You can use it in another function which will perform that check and alert if condition is met. Again, if the conditions are really as you write same all (most of) the time.

function myalert(){
    print("ALERT!!!");
};

function alertifneeded(a, b, c) {
    if ((a || b) && c) {
       myalert();
    }
}

However, if most of the tests are different, there is no sense in creating multiple functions. Then it really will make it less readable with no benefits - if you have each if statement different, there is no problem in changing it. But even then I'd try to figure out some general function which will be able to handle all of the situations, if it is possible...

Upvotes: 0

Jason Hughes
Jason Hughes

Reputation: 778

Assuming your example is just an example and not the real scenario, then yes generally speaking I would do it the first way, that way you can reuse the function, say if ((a > b) && (c < a)) { myfunction(); } Otherwise your code ends up with 50 different functions doing essentially the same thing and just having different logic in them and it becomes a pain in the but to track down which one you need to track down to debug.

Upvotes: 0

Related Questions