0

Well this is pretty basic, but I couldn't find the answers alone. So I hope you guys can help me.

I was trying to bind an event to an array of object, but somehow the variable that got passed is the last one. After it got iterated.

for (var i = 0; i < map_object.length; i++) {
    map_object[i].on('mousedown', function(){
        var x = i;
            setEvent(this, x);
        });
};

function setEvent(data, i){
    console.log(data);
    var x = i;
    console.log(x);
}

the data got passed by value so it always return dynamically, meanwhile (i variable) got passed as reference.

So when it executed I always get this result

data = (some object) {expected result}

i = 9 {unexpected} = map_object.length

I can't figure if this is because (i) got passed as reference, or its because the live event is executed after iteration completed.

the on function come from Fabricjs

Community
  • 1
  • 1
Dimas Satrio
  • 269
  • 3
  • 17

5 Answers5

1

i find [].forEach to be much cleaner to use in these situations, and it reduces closure by not having an inner function in a loop:

[].forEach.call( map_object, function(ob, i){ // replace loop with forEach
    ob.on('mousedown', function(){
        var x = i;
        setEvent(this, x);
    });
});

function setEvent(data, i){
    console.log(data);
    var x = i;
    console.log(x);
}

to me, forEach() just feels less "bolted-on" than the rigmarole imposed by scope-less for loops to defer functions...

EDIT:

if you have jQuery, or something like it, there's actually a better way to do this without a loop:

$("body").on('mousedown', function(){
     var x = map_object.indexOf(this);
     if(x > -1) setEvent(this, x);
});

function setEvent(data, i){
    console.log(data);
    var x = i;
    console.log(x);
}

this does the same thing, but allows you to dynamically add/remove items after the initial run, something you can't do with a run-once for-loop and individual event handlers.

dandavis
  • 16,370
  • 5
  • 40
  • 36
  • Thank you so much for your answer, your trick is something that i've never seen before. Might be the best to vote this as an answer so other can see it too. – Dimas Satrio Dec 09 '14 at 08:32
0

The code executes too late, try this instead:

for (var i = 0; i < map_object.length; i++) {
    prepareSetEvent(i)
};

function prepareSetEvent(i) {
    map_object[i].on('mousedown', function(){
        var x = i;
            setEvent(this, x);
        });
}

function setEvent(data, i){
    console.log(data);
    var x = i;
    console.log(x);
}

The value of i will be copied (as a primitive) when calling the other function. The copy will be used to set the event.

Meligy
  • 35,654
  • 11
  • 85
  • 109
0

By the time the Event fires the loop is completed, so the mousedown Event looks for the last place i was. Since i is scoped globally, it's at the end of the loop by then. To avoid this you use a closure. The same thing occurs, but since the scope comes within the self-executing closure i is where you want it to be.

for(var i=0,l=map_object.length; i<l; i++) {
  (function(i){
    map_object[i].on('mousedown', function(){
      setEvent(this, i);
    });
  })(i);
}

By the way, in jQuery you should use $.each() or $('.someclass').each() instead.

StackSlave
  • 10,613
  • 2
  • 18
  • 35
  • That's correct, but naming `i` the function parameter leads to confusion, could you please edit your answer and rename it? – Jaime Agudo Dec 08 '14 at 23:59
0

The key is to understand when the function arguments are evaluated.

see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/bind

for (var i = 0; i < map_object.length; i++) {
    map_object[i].on('mousedown', setEvent.bind(this, i));
};
Jaime Agudo
  • 8,076
  • 4
  • 30
  • 35
  • 1
    would that bind the element as _this_ or the global as _this_ ? – dandavis Dec 09 '14 at 00:05
  • Good point, I didn't care about `this`, I wanted to show a short solution. You probably want-need the *element* as `this`, but anyway the `setEvent` method doesn't care about the `this` value :P – Jaime Agudo Dec 09 '14 at 00:12
  • for event handlers, it's often needed, and sadly bind() breaks the ability to use _this_ later in the event to get the calling element. we still can reach the "old this" via arguments[0].target in the event handler, so it's not a deal-breaker, but it does require a slight downstream adjustment in the event handler... – dandavis Dec 09 '14 at 00:17
  • Yeah, I am aware but its good you mention it to instruct newcomers, cheers – Jaime Agudo Dec 09 '14 at 01:37
0

You have just been bitten by how Closure actually works. In your event handler function

var x = i; 

x will always have the same value (size of the array - 1). You need to enclose the handler in a function and pass the variable parameter into the function.

for (var i = 0; i < map_object.length; i++) {
  (function(index) {  
    map_object[i].on('mousedown', function(){
            setEvent(this, index);
        };
  })(i);
};

The difference is very subtle. But basically the principle is that, when you assign functions as event handlers, you have to make sure they don't share state with the outside world.

gprasant
  • 15,589
  • 9
  • 43
  • 57