1

Possible Duplicate:
Javascript closure inside loops - simple practical example

I am trying to do something like this:

var obj = function(){
    var members = ['a','b','c','d','e'];
    for(var i in members){
       inst[i] = function(){
           console.log(i);
       }
    }
};

But when I do:

var inst = new obj();
inst.a(); inst.b(); inst.c(); inst.d(); inst.e();

It prints out e 5 times! Also, I would likely to be able to reference this rather than using the name that I am instantiated the object as, but I cannot get that to work at all. The logic behind this is to use websockets to generate an API template client side. This is a rough demo that does basically the same type of thing (logically) but I am getting caught up in this technical issue and it is keeping me from progressing.

Thanks

EDIT

Let me give the complete issue. I didn't want to overload everyone but these solutions aren't working...

var SocketClient = function(config){
//platform specific stuff
var self = this;
var count = 0;
var socket = io.connect(config.address+':'+config.port);
var members = [];
socket.on('method',function(data){
    console.log(data.name);
    KovarApp.client[data.name] = addLogicMember(data.name);
    console.log(KovarApp.client[data.name]);
});
var addLogicMember = function(n){
    return function(config){
        var callback = null;
        if(typeof config.callback !== 'undefined'){
            var callback = config.callback;
            delete config.callback;
        }
        call({
            method: n,
            data: config,
            callback: callback
        });
    };
};
var call = function(config){
    var c = count++;
    var timer = null;
    socket.on('reply_'+c,function(data){
        if(typeof config.callback === 'function'){
            if(timer != null){
                timer.clearTimeout();
            }
            config.callback(data);
            timer = setTimeout(function(){
                socket.removeAllListeners('reply_'+c)
            },10000);
        }
    });
    config.data.handle = c; //make sure that the server has the handle for this function call
    socket.emit(config.method,config.data);
};

};

The server emits 5 functions to the client: login being the first function and getpatientinfo being the last function. On the server, I have it set up to console.log every reply. If I call any of the functions, it console.log's getpatientinfo because that is the string that it receives, meaning that on the client every single function is referencing the last received string (getpatientinfo). I have changed it according to some of the answers, but it still isn't working. Socket shouldn't be an issue here. The issue is obviously my front-end logic when generating members, as every member reflects the last added member.

Solved! See below! (THIS WASN'T CLOSURE!!!)

Community
  • 1
  • 1
Andrew Rhyne
  • 5,060
  • 4
  • 28
  • 41
  • 2
    Your actual problem is a scope problem that has been asked many times (find a duplicate, anyone?), but I wanted to say as an aside that you shouldn't use `for..in` loops over arrays. They loop over all enumerable properties of an object, not merely all elements of an array. – apsillers Dec 11 '12 at 18:32
  • Ya no worries. Its actually using a socket listener to fire the member addition. Its not actually looping through an array; I just wanted to demo the logic easily. – Andrew Rhyne Dec 11 '12 at 18:34
  • This isn't a duplicate. That solution does not work for me. My function is adding members to itself dynamically when invoked from the server, no adding functions with an external loop. – Andrew Rhyne Dec 11 '12 at 18:54

4 Answers4

6

This is called 'a closure effect': each of your functions knows that it should deal with variable named i, but will take its current value when invoked.

To solve this, scope i as well:

var makeMeAFunction = function(x) {
  return function() { console.log(x); };
};
for(var i in members){
   inst[i] = makeMeAFunction(i);
}

As a sidenote, it's usually a bad idea iterating over an Array with for...in: use forEach or, if you just have to support IE8, resort to traditional for(init; check; step) operator.

raina77ow
  • 103,633
  • 15
  • 192
  • 229
1

This should do the trick:

function Cn () {
    var self = this;
    [ 'a','b','c','d','e' ].forEach(function ( name, i ) {
        self[ name ] = function () {
            console.log( i );
        };
    });
}

Note: ES5-shim for IE8

Šime Vidas
  • 182,163
  • 62
  • 281
  • 385
  • 3
    Might be worth noting that this won't work in IE8 or earlier, because of lack of support for Array.forEach. – jevakallio Dec 11 '12 at 18:33
1

You've got a closure related issue - take a look at this for a good write-up.

Kwal
  • 1,531
  • 7
  • 11
0

Found the problem, and boy do I feel like a dolt!

The issue was server side. I was doing this:

  for(var i in functions){
            socket.on(i,function(data){functions[i](data,socket,mysql)}); //create socket listeners
        }

When I changed it to this:

        socket.on('login',function(data){functions.login(data,socket,mysql);});
        socket.on('logout',function(data){functions.logout(data,socket,mysql);});
        socket.on('getVisitInfo',function(data){functions.getVisitInfo(data,socket,mysql);});

it worked great. It looks like it didn't like looping through an object. Oh well, I guess static socket declarations it is!

Andrew Rhyne
  • 5,060
  • 4
  • 28
  • 41
  • While you kept insisting on that 'no closure involved', in fact the problem in the first snippet is _exactly_ the closure one. See, each callback function (event handler) in `socket.on(prop, callback)` call is _defined_ as the one that knows about `i` (for this `functions[i]` line) BUT will use the _current_ value of `i` when actually invoked. Now compare this situation with the one described in my answer. ) – raina77ow Dec 12 '12 at 10:08
  • lol, yup, you are correct! – Andrew Rhyne Dec 12 '12 at 14:38