set Timeout and for in loop

Question

function createDivs(){ 
var styleHash = {};
vertical=0;
horizontal=0;
var h;
var aDiv
var colour;
var groupStyle;
    for(key in elHash){ 
      h=elArr[0][zIndex[key]]/elHash[key];
      colour = randomColor();
      setLocation(h,elHash[key]);
      var container = document.getElementById('container');
      styleElements(container,'width',(scrnWidth-40)+'px');
      styleElements(container,'height',(scrnHeight-200)+'px');
      aDiv = implementDOMelement(container,'div', '');
      groupStyle = function() {       
          styleElements(aDiv ,vposition,vertical+'px');
          styleElements(aDiv ,hposition,horizontal+'px');
          styleElements(aDiv ,'backgroundColor', colour);
          styleElements(aDiv ,'width', elHash[key]+'px');
          styleElements(aDiv ,'height', h+'px');
          styleElements(aDiv ,'zIndex', zIndex[key]);
          if (colour =='#ffffff'){styleElements(aDiv ,'border', 'solid');}
      }
      setTimeout( groupStyle ,1000);
    }
}

function randomColor(){
   var colorR;
   var colorG;
   var colorB;
   colorR = randomNumber(0,255);
   colorG = randomNumber(0,255);
   colorB = randomNumber(0,255);
   return 'rgb('+colorR+','+colorG+','+colorB+')';
}

function implementDOMelement(parentNode, elementType,innHTML, attributes ){//
    var element = document.createElement(elementType);
    for (key in attributes){
      element.setAttribute(key,attributes[key]);
    }
    element.innerHTML = innHTML;
    parentNode.appendChild(element);
    return element;
}

function styleElements(aNode,cssProperty,cssVal){
    aNode.style[cssProperty]=cssVal;
}

Why is 'setTimeout' executed only once instead on every iteration? Well my goal is to pop a div on every sec! Did't put all of the code but it works fine without setTimeOut and groupStyle(code not in a function)

10x for your help , BR

I guess I don't get it either, but let me put all of the code — kidwon, Jun 08 '11 at 22:24

score 3 · Answer 1 · answered Jun 08 '11 at 22:25

It is executed on every loop, but all 10 of them are executed at the same time. The loop goes through the list very fast, and by the time its done looping, not a single timeout has probably occured.

If you are looking to have the aFunc executed every 1 seconds, then use setInterval or alternatively increment the setTimeout time by 1000 after each iteration.

score 2 · Answer 2 · answered Jun 08 '11 at 22:25

2

*psychic debugging* ...I believe you mean this:

var i = 0;

for(key in hash){
  var aFunc = function() {}
  setTimeout(aFunc, 1000 * i++);
}

Your function calls all happen immediately after each other because the for loop takes no time to run and therefore all timeouts are set to approximately the same time. You need to increase the timeout with every iteration to get a once-every-second effect. Or you use setInterval()/clearInterval().

answered Jun 08 '11 at 22:25

Tomalak

332,285
67
532
628

I've put a part of my original code and what I'm after maybe that will give an extra clue – kidwon Jun 08 '11 at 22:31
@kidwon You may be missing the fact that `setTimeout()` returns *immediately* - it does not wait at all. The payload function will be placed on a stack and executed separately when the given time has passed. – Tomalak Jun 08 '11 at 22:34
I see a few weird parts on your code, like "elHash" is never defined o_O (I will assume it to be a global variable). But other than that, Tomalak's answer seems to fit your intent. Give it a try. – Katsuke Jun 08 '11 at 22:36
Well what should I do , I really have to address the hash elements so I need the for in loop? Is there any solution? – kidwon Jun 08 '11 at 22:37
@Katsuke I did but unfortunately it didn't help. elHash is defined but don't want to put extra code even now it's hard to read and it works just fine but I don't want all of the divs to show but to pop-up one by one so here comes the trouble – kidwon Jun 08 '11 at 22:39
@kidwon have you tried to use chrome developer tools and step through the execution? there might be an error somewhere. Also i noticed that when i tested the initial code, key was always the same value (being the last value of the array), given elHash = ['a', 'b', 'c']; the setTimeout always triggers with c, which might be why you see only 1 div "popping" it probably is popping X amount of times, it just happens to be the same div. I will add more after i look a bit into that. – Katsuke Jun 08 '11 at 22:52
1

The `setTimeout()` actually takes the last computed function... Even if you put it in an array like [this](http://jsfiddle.net/passcod/jBcG3/0/). I'm currently trying several things to get it to do what we want, but it either does that same (wrong) thing or fails silently... frustrating! An interesting issue nonetheless. – Félix Saparelli Jun 08 '11 at 22:59
@passcod This is a scoping issue. `setTimeout` takes a function *reference*, not a copy. If you've been overwriting this reference in a loop, then it's no surprise they all are the same after the loop. A closure solves this problem. – Tomalak Jun 09 '11 at 05:52
Also, and closer to my last fiddle, there is a way without a (explicit) closure: create a class which contains a `var` set to the value you want to use, and a method which is the actual function to be executed. Then instantiate that class and push the resulting object to an array. You can then pass that array's element to `setTimeout`... and it works. – Félix Saparelli Jun 09 '11 at 08:23
@passcod: Technically, that's a closure as well. ;) – Tomalak Jun 09 '11 at 08:45

score 1 · Accepted Answer · edited May 23 '17 at 12:12

Look at this, it might just be the answer you are looking for:

setTimeout in for-loop does not print consecutive values

In your case: aDiv is pointing always to the last div of the loop. This is my guess why it looks like it only triggers once.

a simple solution for your code should be along this lines:

  groupStyle = function() {       
      styleElements(aDiv ,vposition,vertical+'px');
      styleElements(aDiv ,hposition,horizontal+'px');
      styleElements(aDiv ,'backgroundColor', colour);
      styleElements(aDiv ,'width', elHash[key]+'px');
      styleElements(aDiv ,'height', h+'px');
      styleElements(aDiv ,'zIndex', zIndex[key]);
      if (colour =='#ffffff'){styleElements(aDiv ,'border', 'solid');}
  }
  setTimeout( groupStyle ,1000);
  //replaced with:

  // I will assume vposition and hposition were supposed to be strings not variables
  doGroupStyle(aDiv, vertical, horizontal, elHash, key, zIndex, colour);

  // then create the doGroupStyle function
  function doGroupStyle(aDiv, vertical, horizontal, elHash, key, zIndex, colour) {
    setTimeout(function() {
      styleElements(aDiv ,'vposition',vertical+'px');
      styleElements(aDiv ,'hposition',horizontal+'px');
      styleElements(aDiv ,'backgroundColor', colour);
      styleElements(aDiv ,'width', elHash[key]+'px');
      styleElements(aDiv ,'height', h+'px');
      styleElements(aDiv ,'zIndex', zIndex[key]);
      if (colour =='#ffffff'){styleElements(aDiv ,'border', 'solid');
      }, 1000 * key);
    }
  }


// or an alternative approach as passcod suggested:
(function(aDiv, vertical, horizontal, colour, elHash, key, h, zIndex) {
groupStyle = function() {       
    styleElements(aDiv ,'vposition',vertical+'px');
    styleElements(aDiv ,'hposition',horizontal+'px');
    styleElements(aDiv ,'backgroundColor', colour);
    styleElements(aDiv ,'width', elHash[key]+'px');
    styleElements(aDiv ,'height', h+'px');
    styleElements(aDiv ,'zIndex', zIndex[key]);
    if (colour =='#ffffff'){styleElements(aDiv ,'border', 'solid');}
};
setTimeout(groupStyle ,1000 * key);
})(aDiv, vertical, horizontal, colour, elHash, key, h, zIndex);

Haven't tested it so you maybe have to modify it a bit. But that's the idea behind it.

Hey, what if `key` isn't numeric? Well, I found that [using an inline closure](http://jsfiddle.net/passcod/jBcG3/2/) works. It's derived from the answer you linked to. — Félix Saparelli, Jun 08 '11 at 23:17
@passcod if key is not numeric then you make your own counter :) I assumed it was because he said he used Tomalak's suggestion which required it to be numeric. Also his request for the divs to "pop a div on every sec". Your solution is the first thing i came across :) very clean but it all happens at once. — Katsuke, Jun 08 '11 at 23:23
@passcod [here](http://jsfiddle.net/U5Nab/) is a version of yours with the counter added. And thanks for showing me http://jsfiddle.net/ didn't know about it ^^ — Katsuke, Jun 08 '11 at 23:24
Oh right, I misread that comment. Sorry! Also, you forgot the `i` in the function definition in that last fiddle... [here it is](http://jsfiddle.net/passcod/U5Nab/1/) fixed. — Félix Saparelli, Jun 08 '11 at 23:26

set Timeout and for in loop

3 Answers3