0

I am trying to build up an array of handlers for a 3rd party control. One of the required objects inside is a callback handler. The problem is the allProvider(item) is called, the "theValue" variable no longer points to the same location - it always is set to the last value it was assigned to. What I'd really like to do is evaluate the "theValue.name" immediately when building the array instead of later. I'm open to about any solution at this point. Thanks

var handlers = [];
for(var i=0;i<myArray.length;i++){
    var theValue = myArray[i];
    handlers.push({ 
        name: myArray.name,
        allProvider: function(item){
            return "All "+ theValue.name; //This always == myArray[myArray.length - 1]
        }
    });
 }
John P
  • 1,540
  • 2
  • 22
  • 34

3 Answers3

1

You have a closure there - theValue is a reference to the parent scope's variable, and by the time that function is executed it is pointing to the last value it was assigned to.

You need to break the closure. A common pattern is to use an IIFE.

allProvider: (function(theValue) {
               return function(item) {
                 return "All "+ theValue.name;
               }
             })(theValue)
alex
  • 479,566
  • 201
  • 878
  • 984
  • That worked perfectly. You get the check for both being first and for the article - exactly what I needed. Thanks – John P Jan 12 '15 at 03:51
1

Alex is correct, one possible way of solving this closure problem is to try this:

var handlers = [];

myArray.forEach(function (theValue) {
    handlers.push({ 
        name: myArray.name,
        allProvider: function(item){
            return "All "+ theValue.name;
        }
    });
});

Inside each anonymous function called within the forEach method, theValue will always point to what you expect.

EDIT: I wasn't thinking of something more efficient than what you had, just something that worked. map is definitely hitting two birds with one stone, so-to-speak, so you can also try this:

var handlers = myArray.map(function (theValue) {
    return { 
        name: myArray.name,
        allProvider: function(item){
            return "All "+ theValue.name;
        }
    };
});

Lastly, you can try this if you don't want something that could potentially break in older browsers that don't support forEach or map:

var handlers = [],
    i;

for(i = 0; i < myArray.length; i++) {
    (function (theValue) {
        handlers.push({ 
            name: myArray.name,
            allProvider: function(item){
                return "All "+ theValue.name;
            }
        });
    }(myArray[i]));
}
Patrick Roberts
  • 49,224
  • 10
  • 102
  • 153
1

The problem is by the time the callback function is being called, the for loop is finished, So at the calling time theValue refers to last element in array.

I'll suggest you to use native array map function for your work like bellow:-

var handlers = myArray.map(function(theValue) {
    return {
        name: myArray.name,
        allProvider: function(item) {
            return "All " + theValue.name;
        }
    };
});

BTW IE8 doesn't support the map function.

If you want to support IE8 and lower What you can do bind each element in array to a function like bellow.

var getObject = function(arrName, valueName) {
    return {
        name: arrName,
        allProvider: function(item) {
            return "All " + valueName; 
        }
    }
}

var handlers = [];
for (var i = 0; i < myArray.length; i++) {
    var theValue = myArray[i];
    handlers.push(getObject(myArray.name,theValue.name));
}
Mritunjay
  • 25,338
  • 7
  • 55
  • 68