1

I have a problem with my code bellow:

for (var i =0; i < filterArray.length; i++)
{

    name1 = filterArray[i];
    selectstring = 'a[cat="'+name1+'"]';
    filterMenuItem.addItem(name1 , function() {$(selectstring).show(); alert(selectstring)});


}

filterArray contains 2 items currently.. "Foo" & "Bar"..

now this "addItem(name1" works fine. We get an added menu item with the name Foo and another named Bar

the issue comes with the next part (the function).

both the Foo & Bar menu items functions end up doing the same thing. (e.g. they both get the select string 'a[cat="bar"]'... as tested with the alert)

Now i assume this is happening because the selectstring variable is being passed by reference to the function? So when i'm setting selectstring value a second time it is overriding the value from the first loop?

How can I pass a unique copy of selectstring to a selector? i tried "function(selectstring) {....}" but this didn't help..

Thanks for anyone who can shed some light on the subject!

EDIT: Revised code below.. Still same issue:

for (var i =0; i < filterArray.length; i++)
{

    var name1 = filterArray[i];
    var selectstring = 'a[cat="'+name1+'"]';

    filterMenuItem.addItem(name1 , function() {alert(selectstring); $('.sortablelist').hide();  $(selectstring).show();});


}
james
  • 1,031
  • 1
  • 15
  • 31

3 Answers3

1

It has nothing to do with "passing by reference" to the function -- nothing is passed to the function, actually.

I think the problem is the variable scope. You didn't use var before selectstring on the assignment line, so it's assumed to be a global variable. The anonymous function will keep a reference to this global variable, which will have the value of 'bar' by the time the function is run. If you use var selectstring = (...), a separate closure will be created on each iteration, and it should work.

EDIT

I answered too fast, and the answer was wrong, so here is an updated solution. I assumed a closure was being created when the anonymous function was declared, but that's not the case. So the actual solution involves creating a closure to retain the variable's value for each loop iteration:

for (var i=0; i < filterArray.length; i++) {
    addFilteredMenuItem(filterArray[i]);
}

function addFilteredMenuItem(itemName) {
    var selectstring = 'a[cat="'+itemName+'"]';
    filterMenuItem.addItem(itemName, function() {
       alert(selectstring);
    });
}

Working example on jsfiddle

The solution I posted as a comment below also works, and is based on the same principle, but I think it looks bad for involving an unnecessary self-executing function.

See also this SO thread discussing the same subject.

Community
  • 1
  • 1
bfavaretto
  • 71,580
  • 16
  • 111
  • 150
  • thank for the response bfavaretto.. But I originally had both variables with var before them when I originally wrote this.. I changed to moving the var declarations outside of the loop to see if this fixed my issue when it first came up.. I've put the var's back inside the loop but i'm still getting the same issue.. – james Nov 25 '11 at 16:27
  • 1
    I managed to find a [working solution using an outer closure](http://jsfiddle.net/3yr3r/4/). It looks extremely ugly, but works. – bfavaretto Nov 25 '11 at 16:44
  • thank you very much, worked perfectly. If you could edit your post above with the correct info i'll mark it as the correct answer.. Thanks again! – james Nov 25 '11 at 16:52
1

Your for loop only has one closure, and so the 'latest' value of selectstring will always be used. You need to start a new closure as below:

for (var i =0; i < filterArray.length; i++)
{
  name1 = filterArray[i];
  selectstring = 'a[cat="'+name1+'"]';
  filterMenuItem.addItem(name1, 
    (
     function(selectstring) 
     { 
       return function() {$(selectstring).show(); alert(selectstring) }
     }
    )(selectstring)
  );
}

The function gets called immediately while the correct value of selectstring is still there, and a new closure is created by the outer function (and thus a new copy of its local variables) for the returned inner function.

The jQuery .each() function could also be quite a bit simpler to use in your case:

$.each(this.filterArray, function(i, filter) {
    filterMenuItem.addItem(name1 , function() {$('a[cat="'+filter+'"]').show(); alert('a[cat="'+filter+'"]')});
});
Dennis George
  • 1,495
  • 2
  • 10
  • 18
  • thanks for the response! Both yours and bfavaretto answers worked.. Think i need to do some reading on js closures! – james Nov 25 '11 at 17:52
0

I don't know if I know what You need, but try:

for (var i =0; i < filterArray.length; i++)
{

    name1 = filterArray[i];
    selectstring = 'a[cat="'+name1+'"]';
    filterMenuItem.addItem(name1 , function(selectstring) {$(selectstring).show(); alert(selectstring)});

}
David Horák
  • 5,535
  • 10
  • 53
  • 77
  • Thanks for the response but I have tried this but the alert now sees select string as an event? (it now pops up saying (Event) { .... }) – james Nov 25 '11 at 16:22