Javascript For-Loop issue

Question

I'm trying to click every item with a 200 millisecond interval, I wrote the following script however there seems to be an issue with the For Loop. Some one please tell me what you think is wrong with it.

function clickLink(elm) {
    var evt = document.createEvent('MouseEvents');
    evt.initMouseEvent('click', true, false, window, 0, 0, 0, 0, 0, false, false, false, false, 0, null);
    elm.dispatchEvent(evt);
}

function sel() {
    elms = document.getElementsByClassName('uItem');
    var inputs= elms;
    var howbig= elms.length;
    console.log(howbig);

    for (var i=250;i<elms.length;i++)
    {
        setTimeout(clickLink(inputs[i]),200)
    };

There's 1400 uItem 's on the page.

What happens when you run it? What led you to the conclusion that the for-loop is the problem? — Jani Hartikainen
– Jani Hartikainen, Commented Nov 10, 2012 at 14:43
@JaniHartikainen With or without the for loop in the equation all that happens is i get a console message that sais 1400 (the amount of UItem's on the page yet none are clicked. — ModS
– ModS, Commented Nov 10, 2012 at 14:45
You are passing the function's return value to the setTimeout instead of a function reference. — m90
– m90, Commented Nov 10, 2012 at 14:56
you want to click all items at the same time, or clicking each item one by one, spaced 200 ms? — BAK
– BAK, Commented Nov 10, 2012 at 15:14

I Hate Lazy · Accepted Answer · 2012-11-10 15:27:55Z

4

Cleanest solution is just to have your clickLink function return a function.

function clickLink(elm) {
    return function() {
        var evt = document.createEvent('MouseEvents');
        evt.initMouseEvent('click', true, false, window, 0, 0, 0, 0, 0, false, false, false, false, 0, null);
        elm.dispatchEvent(evt);
    };
}

Then stagger the timers if you want an interval:

var start = 250
for (var i=start;i<elms.length;i++) {
    setTimeout(clickLink(elms[i]), 200 * (i - start))
}

Or ditch the loop, and use setInterval:

var i = 250,
    len = inputs.length;
var itvl = setInterval(function() {
    clickLink(elms[i++]);
    if (i >= len)
        clearInterval(itvl);
}, 200);

And to back to your original clickLink function

edited Nov 10, 2012 at 15:27

answered Nov 10, 2012 at 15:16

I Hate Lazy

49k13 gold badges89 silver badges79 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

eh9 Over a year ago

This is the explicit way of returning a bound function, as I mentioned in my answer. My experience with relatively complex landing function for asynchronous operations (in my case it was AJAX, but the flow of control is identical), is that I found the code cleaner when I relied on bind to do the same. Using that call makes it explicit when the arguments are evaluated and kept my head straight more easily.

Nelson Benítez León Over a year ago

You need to better adjust your 200 * i interval, see the UPDATE 3 of my answer.

I Hate Lazy Over a year ago

@Nelson: Yeah, good point. I forgot about the starting point.

Nelson Benítez León Over a year ago

+1 for your solution and the feedback gave to others solution.

guillaume · Accepted Answer · 2012-11-10 15:08:04Z

2

There is a issue of the scope of ì, the delay which needs to be incremented accordingly with i to have a 200 ms interval and the fact that setTimeout expects a function as is argument.

    for (var i=250;i<elms.length;i++)
    {
        setTimeout((function() {
            var j = i // keep i as j in this closure
            return function() { // return the right function
                clickLink(inputs[j])
            }
        })(),200 * i) // set the delay depending on i
    };

edited Nov 10, 2012 at 15:08

answered Nov 10, 2012 at 14:59

guillaume

1,39010 silver badges15 bronze badges

3 Comments

eh9 Over a year ago

The value of i inside the anonymous function is not the i of the loop variable, because the loop is not in scope when the anonymous function executes. Unless i is in scope as a global (shudder), its value will be null.

guillaume Over a year ago

what makes you say i isn't in scope?

I Hate Lazy Over a year ago

i is definitely in a reachable scope of the anonymous function.

Yoshi · Accepted Answer · 2012-11-10 15:27:46Z

2

function sel() {
  var elems = document.getElementsByClassName('uItem');
  for (var i = 0; i < elems.length; i += 1) {
    setTimeout(function (el) {
      return function () {
        clickLink(el);
      };
    }(elems[i]), i * 1000);   
  };
}

demo: http://jsfiddle.net/4hYWy/

little note: I don't like these constructs (calculating the delay like in the example). But it is very close to the original code, and my solution of choice would have probably made it too complicated.

edited Nov 10, 2012 at 15:27

answered Nov 10, 2012 at 15:13

Yoshi

54.8k14 gold badges93 silver badges108 bronze badges

1 Comment

eh9 Over a year ago

I mentioned in my answer that you can make bound functions explicitly. This answer is an example of how that's done, with a function that returns a function.

eh9 · Accepted Answer · 2012-11-10 15:32:48Z

2

The easiest way, if you're using a moderately-recent version of JavaScript, is to use bind. This creates a single function object with arguments bound to it that are calculated when bind is called, not later. The line you want is thus:

setTimeout( clickLink.bind( null, inputs[i] ),  /* delay expression here */ )

You can make bound functions explicitly, but it's ugly and you should only do it if you have a strong requirement to support old JavaScript interpreters.

edited Nov 10, 2012 at 15:32

answered Nov 10, 2012 at 14:58

eh9

7,45824 silver badges44 bronze badges

4 Comments

I Hate Lazy Over a year ago

I like the .bind solution, but you're binding the this value of the function, so you'd need to either pass a different value as the first argument before inputs[i], or you'd need to change the clickLink function to use this instead of elm.

eh9 Over a year ago

In the present example, that doesn't matter, because this isn't used in clickLink. In general, though, I agree that you have to pay attention to the value of this you get.

I Hate Lazy Over a year ago

I agree, this isn't used, and that's the trouble. The elm parameter will be undefined because nothing is passed, and nothing is bound.

eh9 Over a year ago

Oh, you're right. Brain fart. Fixed. I was so used to constructing bound functions explicitly I got the signature of bind wrong in my head.

BAK · Accepted Answer · 2012-11-10 15:41:53Z

2

This code wait 200ms, execute a callback and prepare a new callback executed 200ms later. The second callback is executed 400ms after the call of sel and so on.

    var callback = function(i) {
        return function() {
            if (i < elms.length) {
                clickLink(inputs[i]);
                setTimeout(callback(i + 1), 200);
            }
        };
    };
    setTimeout(callback(250), 200);

edited Nov 10, 2012 at 15:41

answered Nov 10, 2012 at 15:30

BAK

1,0059 silver badges23 bronze badges

2 Comments

I Hate Lazy Over a year ago

Most of the other answers multiply 200 * i so they're not executed at the same time. But take a look at your returned function. You are invoking it immediately, so it never gets passed to the setTimeout. You'll need to remove the trailing () from the returned function.

I Hate Lazy Over a year ago

+1 because I do like this solution. But there's one more problem. The increment of i is done after the comparison to .length, so on the last call, i will be equal to elms.length, which means inputs[i] will be undefined. You should make the clickLink(inputs[i]) conditional as well.

Nelson Benítez León · Accepted Answer · 2012-11-10 15:32:26Z

1

The way your doing this won't get you one click per 200ms, you will register all your clicks almost at the same time(the time a loop last to iterate 1000 items,almost instantly) so you basically would get your 1000+ aprox. click events fired almost simultaneously, I think that could cause issues, so to make sure they are executed once in a 200ms approx. do this:

window.i=250;
for (var i=250;i<elms.length;i++)
{
setTimeout(function() {
   clickLink(inputs[window.i]);
   window.i++;
},200 * (i-249)); //Here we make sure events are spaciated 200ms each approx.
};

UPDATE: ~~Updated the anonymouse function to pass i to avoid closure issue noted in comments.~~

UPDATE 2: Added window.i inside anonymous function to workaround i scope issue

UPDATE 3: Updated my code above to fix the interval at which each click event is fired, we were using 200 * i but that is only correct if i starts at zero, here it starts at 250 so the correct form is 200 * (i-249)

In spite of some comments, the i scope issue it's resolved using window.i , see test here https://tinker.io/98fd4/1

edited Nov 10, 2012 at 15:32

answered Nov 10, 2012 at 14:54

Nelson Benítez León

51k8 gold badges70 silver badges84 bronze badges

9 Comments

eh9 Over a year ago

Added an argument to the function doesn't work either, because when setTimeout calls its argument function, it does so with no arguments. Thus i will always be null.

I Hate Lazy Over a year ago

You can only add the argument if you pass i after the duration. Also, it doesn't work in some browsers to pass arguments to the callback.

Nelson Benítez León Over a year ago

Thanks for spotting the scopping issue, I've made an update to mitigate the issue using a global var inside the anonymous function.

I Hate Lazy Over a year ago

Using window.i is the opposite direction you want to go. You need the scope to be more local, not less.

eh9 Over a year ago

window.i doesn't fail because it fails to function. It fails because using a global for this purpose leads to unreliable code, since the tramples on the most basic principles of encapsulation.

|

Paulo R. · Accepted Answer · 2012-11-10 15:42:15Z

1

No scope issues or calculating correct interval.

function sel() {
    var elms = document.getElementsByClassName('uItem'),
        i = -1,
        I = elms.length,

        rec = function () {
            if ( i++ < I ) {
                clickLinks(elms[i]);
                setTimeout(rec, 200);
            }
        };

    rec();
}

http://jsfiddle.net/joplomacedo/cqfxH/

edited Nov 10, 2012 at 15:42

answered Nov 10, 2012 at 15:29

Paulo R.

15.7k4 gold badges33 silver badges42 bronze badges

Collectives™ on Stack Overflow

Javascript For-Loop issue

7 Answers 7

4 Comments

3 Comments

1 Comment

4 Comments

2 Comments

9 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

7 Answers 7

4 Comments

3 Comments

1 Comment

4 Comments

2 Comments

9 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related