Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Below is a simplified version of some code I am using to execute a function after all asynchronous calls complete. Is this a reasonable piece of code? It seems to work. Nothing would break if DoStuff() was called twice, though that would be very inefficient.

var i = 0;
var done = function () {
    i++;
    if (i < 2) return;
    //Everything is done
    DoStuff();
};
elem1.executeAsync(function () {
    done();
});
elem2.executeAsync(function () {
    done();
});
share|improve this question

closed as off-topic by Jamal Oct 31 at 23:10

This question appears to be off-topic. The users who voted to close gave this specific reason:

If this question can be reworded to fit the rules in the help center, please edit the question.

    
So you want DoStuff() called only when both are done? –  SomeKittens Nov 12 '12 at 15:43
    
@SomeKittens: Yes. –  Brian Nov 12 '12 at 16:18

1 Answer 1

up vote 4 down vote accepted

The approach itself (i.e. using a counter) is reasonable, but the hard-coded 2 and DoStuff limits its use.

You could do something like this, to derive more generic implementation:

function createCounter(count, callback) {
  count || (count = 1); // default to 1
  (typeof callback === 'function') || (callback = function () {}); // default to no-op
  return function () {
    --count || callback();
  };
}

Now you have a function that returns a specialized "done" function. Use like so:

var done = createCounter(2, DoStuff);

elem1.executeAsync(function () {
  done();
});

elem2.executeAsync(function () {
  done();
});

Incidentally, this can be shortened to:

var done = createCounter(2, DoStuff);

elem1.executeAsync(done);
elem2.executeAsync(done);

(of course, your actual code is likely more complex)

However, I'd say you should also look into the Promise/Deferred patterns.
For instance, using jQuery's implementation (and assuming executeAsync returns a promise) you can do this

$.when(elem1.executeAsync(), elem2.executeAsync()).then(DoStuff);
share|improve this answer
    
executeAsync is not under my control. –  Brian Nov 12 '12 at 16:22
    
@Brian Ok, but apart from the last bit about promises, that doesn't affect my answer. –  Flambino Nov 12 '12 at 16:31

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