Javascript for loop function closure printing out of order

Question

I am implementing the IIFE method when wrapping a for a loop around an async/ajax call.

var j = 4;
for (var i = 0; i < j; i++) {
    (function(cntr) {
        asyncCall(function() {
            console.log(cntr);
        });
    })(i);
}

The problem is that when I console.log cntr, I get all of the values, but they have a random order. Let's say I have a for loop from 0-4. It will print these values in a random order, like 2,1,3,4,0. This changes every time I rerun the code.

Edit: The question linked to most certainly is not the answer. Please pay more attention before marking as duplicate. I'm also not even using nodejs...

I am using facebook api, but not necessarily that relevant to the question. It could have been any async call. — D-Marc, Mar 23 '17 at 07:21
if `asyncCall` has random delay - you should get random numbers like in your case. To make it work, I would use Chain Promise — Maxim Shoustin, Mar 23 '17 at 07:21
`cntr`? Looks `undefined` to me. Oh, also you should change `)(i)` at the end of your self-executing function to `(i))`, as some Browsers are not accepting the other way now. — StackSlave, Mar 23 '17 at 07:26
This is the correct way to do function closures, PHPglue http://stackoverflow.com/questions/11488014/asynchronous-process-inside-a-javascript-for-loop — D-Marc, Mar 23 '17 at 07:28
Figured out the answer, but I guess slebetman doesn't want me to share it with the rest of the community. — D-Marc, Mar 23 '17 at 18:59

gyre · Answer 1 · 2017-03-23T07:44:19.047

Your asyncCall doesn't finish in a constant amount of time. Since you begin each asynchronous call within a synchronous for-loop, whichever call finishes first will log first (in other words, the calls do not wait for each other).

There are two ways to go about fixing this problem. The first is to wait for each asyncCall to complete before beginning the next, which you could do easily with recursion:

var calls = 4
var i = 0

asyncCall(function handler() {
  console.log(i)
  if (++i < calls) asyncCall(handler)
})


function asyncCall(handler) {
  setTimeout(handler, 0)
}

This approach makes the most sense if each successive call depends on the result of the previous call, which yours might not. In that case, it is more efficient to have each call execute up-front, but store the results in an array which you log once every call has completed:

var calls = 4
var done = 0
var i = 0

var results = []

for (var i = 0; i < calls; i++) (function(i) {
  asyncCall(function handler() {
    results[i] = i // example data
    if (++done === calls) complete()
  })
})(i)

function complete () {
  // Do something with your array of results
  results.map(function (e) {
    console.log(e)
  })
}


function asyncCall(handler) {
  setTimeout(handler, Math.random()*10)
}

StackSlave · Answer 2 · 2017-03-23T08:12:51.133

A self-executing closer, which it looks like you're looking for, is used like this:

//<![CDATA[
/* external.js */
var doc, bod, htm, C, T, E; // for use onload elsewhere
addEventListener('load', function(){

doc = document; bod = doc.body; htm = doc.documentElement;
C = function(tag){
  return doc.createElement(tag);
}
T = function T(tag){
  return doc.getElementsByTagName(tag);
}
E = function(id){
  return doc.getElementById(id);
}
addClassName = function(element, className){
  var rx = new RegExp('^(.+\s)*'+className+'(\s.+)*$');
  if(!element.className.match(rx)){
    element.className += ' '+className;
  }
  return element.className;
}
removeClassName = function(element, className){
  element.className = element.className.replace(new RegExp('\s?'+className), '');
  return element.className;
}
var outs = doc.getElementsByClassName('output');
for(var i=0,out,l=outs.length; i<l; i++){
  (function(){
    var out = outs[i], b = false;
    out.onclick = function(){
      if(b){
        removeClassName(out, 'blue'); b = false;
      }
      else{
        addClassName(out, 'blue'); b = true;
      }
    }
  }());
}

}); // close load
//]]>

/* external.css */
html,body{
  padding:0; margin:0;
}
.main{
  width:980px; margin:0 auto;
}
.output{
  width:100px; border:1px solid #000; border-top:0;
}
.output:first-child{
  border-top:1px solid #000;
}
.blue{
  background:lightblue;
}

<!DOCTYPE html>
<html xmlns='http://www.w3.org/1999/xhtml' xml:lang='en' lang='en'>
  <head>
    <meta http-equiv='content-type' content='text/html;charset=utf-8' />
    <link type='text/css' rel='stylesheet' href='external.css' />
    <script type='text/javascript' src='external.js'></script>
  </head>
<body>
  <div class='main'>
    <div class='output'>Sorry</div>
    <div class='output'>This</div>
    <div class='output'>Example</div>
    <div class='output'>Isn&#039;t</div>
    <div class='output'>Better</div>
  </div>
</body>
</html>

Here's how the closure works. Events actually fire when they occur, like onclick. By the time the Event fires you are actually at the end of the loop, so when that happens, vars and arguments look for the level they were last scoped to.

Javascript for loop function closure printing out of order

2 Answers2