1

Possible Duplicate:
Passing functions to setTimeout in a loop: always the last value?

I have the following code:

var points = [{
        id1             : 1,
        id2             : 9,
        lat             : 44,
        lng             : 24
    },{
        id1             : 2,
        id2             : 7,
        lat             : 13,
        lng             : 29
    }];

when I am trying to create timeouts for every object in the array points, it updates only the last element

for (var i in points){
    setInterval(function(){
        drivePoint(points[i], i)
    }, 1000);
}

where drive point is the function that makes ajax request:

function drivePoint(point, i){
        $.ajax({
            type: "POST",
            url: 'url',
            data: points[i]
        }).done(function(o){
            var data = $.parseJSON(o);
            points[i].lat           = data['lat'];
            points[i].lng           = data['lng'];
        });
    }

idea of drivePoint is to update the coordinate of a given point, but the problem is that it updates only the last one, nevertheless how many objects are in points variable if instead of for (var i in points) loop to write this separately

setInterval(function(){
    drivePoint(points[0], 0)
}, 1000);

setInterval(function(){
    drivePoint(points[1], 1)
}, 1000);

everything works fine

Can not understand where is the problem

Community
  • 1
  • 1
Salvador Dali
  • 214,103
  • 147
  • 703
  • 753
  • This question is a contestant for "most confusing thing about JavaScript programming" :-) – Pointy Sep 08 '12 at 22:14
  • The problem is that `for...in` is for objects not arrays, and `points` is an array (of objects). You need to use a regular `for` loop plus what **Pointy** said. – elclanrs Sep 08 '12 at 22:16
  • Note, I think a `setTimeout()` may work better with such a call, since it runs once. So, set a drive point, set timeout, set the next, etc., until it finishes. This can be done with a closure-scoped variable containing the iteration and list. – Jared Farrish Sep 08 '12 at 22:26
  • See: http://jsfiddle.net/userdude/Bw3kb/ – Jared Farrish Sep 08 '12 at 22:40
  • Another take: http://jsfiddle.net/userdude/Bw3kb/1/ – Jared Farrish Sep 08 '12 at 23:01

1 Answers1

1

The problem here is that when your function is executed i will have been set to the last value in the loop, therefore that will be used when executing drivePoint(). But do you really need seperate intervalls for each point? Could you just have your interval loop through the all values instead?

setInterval(function() {
    for (var i in points) {
        drivePoint(points[i], i);
    }
}, 1000);
Karl-Johan Sjögren
  • 16,544
  • 7
  • 59
  • 68
  • Look at it again; unless I'm reading it wrong, each drive point should be called a second apart, where what you have does all points in a loop, then iterates, then does it again? – Jared Farrish Sep 08 '12 at 22:25
  • I would guess what the OP is trying to do is update the points one every second, not update all the points every second. If so, a for loop with `setTimeout()` _inside_ the loop is what is needed. – nnnnnn Sep 08 '12 at 22:25
  • @JaredFarrish: Karl-Johan's code does nearly the same as that in the question. Take another look at the original. The intervals are starting at basically the same time, and they have the same duration, which means they will execute at basically the same time. But I'm guessing @ nnnnnn is right, and this is not what is actually wanted. – gray state is coming Sep 08 '12 at 22:30
  • @graystateiscoming - Umm, me and nnnnnn are saying the same thing... – Jared Farrish Sep 08 '12 at 22:31
  • @JaredFarrish: I'm saying that OP's code isn't timed in the manner that you and @ nnnnnn are describing, but Karl-Johan's code does effectively emulate what OP's original code timing, (but of course fixes the `i` issue). – gray state is coming Sep 08 '12 at 22:33
  • @graystateiscoming - So I'm wrong and nnnnnn is right? The point is, "making the code work" does not, in this case, "fix the problem". I think the OP wants to do something like: http://jsfiddle.net/userdude/Bw3kb/ But that's up for conjecture. – Jared Farrish Sep 08 '12 at 22:39
  • @JaredFarrish: Ugh! ;-) Reading your first comment, you said to look at it again because each drive point should be called a second apart. But if you do look at it, you'll see they should *not* be called a second apart. They should be called at the same time (after 1 second). I'm just saying that @ nnnnnn's *speculation* may be correct that the staggered behaviour is actually wanted. But then again, look at the last code example, which according to OP provides the correct behavior. In that code, they will also be at the same time, and not staggered. – gray state is coming Sep 08 '12 at 22:44
  • @graystateiscoming - I'm sure that Jared's initial comment was describing what he thought was _required_, not what the OP's current code actually does. That is, he _was_ making the same point as me... – nnnnnn Sep 08 '12 at 22:48
  • @graystateiscoming - That's an interpretation. If that is correct, and there's multiple points and multiple lines or updates to the same line, then doing an iteration within the current iteration is correct. I don't read it that way, but it could be correct. Relax, though; it's ok. – Jared Farrish Sep 08 '12 at 22:49
  • @nnnnnn: Apparently you're right. His wording didn't make it sound like that. Anyway, the desired behaviour seems to be illustrated by the last code example, except presumably for the interval. – gray state is coming Sep 08 '12 at 22:52
  • @graystateiscoming - This, of course, is what you're thinking it should be: http://jsfiddle.net/userdude/Bw3kb/1/ – Jared Farrish Sep 08 '12 at 23:00
  • @JaredFarrish: I've no idea at this point. ;-) – gray state is coming Sep 08 '12 at 23:08
  • If you read the original post he does say that he wants to create a timeout/interval for each object but his sample doesn't show that and he gives no reason for doing it so thats why I suggested this instead. The thread was closed anyway but thanks for the nice discussion! – Karl-Johan Sjögren Sep 09 '12 at 07:34