0

consider:

for (var i in somecollection){
   var a = document.createElement('a');
   a.onclick = new function(){callSomeMethod(somecollection[i]);};
   ...
}

At runtime, all 'a' elements wind up calling callSomeMethod with the same parameter value (the last element in 'somecollection'.

I have a hack of a solution as follows:

for (var i in somecollection){
   var a = document.createElement('a');
   var X = 'callSomeMethod(\''+somecollection[i]+'\');';
   a.setAttribute('onclick', X);
   ...
}

But this forces me to exclude 'callSOmeMethod' from mangling/compression when I minify my JS files. How can I make each 'a' element's click handler callSomeMethod with a different parameter without hardcoding the function name in a string?

The closest my search found is the accepted answer in pass string parameter in an onclick function but I do not know how to create a 'scope bubble' .

Thanks...

Community
  • 1
  • 1
adaj21
  • 543
  • 3
  • 11
  • 25

4 Answers4

1

You can use a closure, it will capture the value of i

for (var i in somecollection){
    var a = document.createElement('a');
    a.onclick = (function(index) {
        return function () {
            callSomeMethod(someCollection[index])
        };
    })(i);
    ...
 }

That way, the correct value of index will be available when the function is called, but it won't be called until the onClick event fires.

tlehman
  • 5,125
  • 2
  • 33
  • 51
  • 1
    Where's the closure? This is exactly the same example as the first snippet in the question :-( (though without `new`). – Teemu Dec 29 '13 at 17:42
  • @Teemu the `function() { ... }` expression is the closure, since it has the free variable `i` that closes over the environment. Also, notice that the OP is using `new`, that's not correct. – tlehman Dec 29 '13 at 17:44
  • 1
    Please test your answer, there's no closure in it. The only difference with OP's code is that you've dispersed the code to separate lines... The `new` needs to be omitted, ofcourse. – Teemu Dec 29 '13 at 17:45
  • @TobiLehman: `i` is defined in the head of the loop, and its scope is the outer function (or global) scope. The `function() {...}` creates a closure over that outer scope, so all functions created in the loop share the same `i` variable. As such, this isn't a correct fix. – cookie monster Dec 29 '13 at 18:04
  • @cookiemonster @Teemu you are right, I missed that, I used an IIFE to wrap the function call, this way the current value of `i` gets captured, and then the returned function closes over that value. – tlehman Dec 29 '13 at 19:05
  • 1
    Yep, this is an answer now ; ). – Teemu Dec 29 '13 at 19:12
1

You could use the power of javascript ! juste add custom property to the object.

Here is a example:

var somecollection= [ 'a','b','c','d'];

function callSomeMethod() {
     var i = this.__index; // retreive here your data
     if (i) {
        alert(somecollection[i]);
     }
}

function init() {

    for (var i in somecollection){
        var a = document.createElement('a');
        a.onclick = callSomeMethod;
        a.innerHTML = "click for #" + i;
        a.__index = i; // custom property to capture index or any data you want to pass
        document.body.appendChild(a);    
    }
}
Sayris
  • 1,060
  • 10
  • 10
  • Thanks. Quite an interesting approach. However, in my case, somecollection is transient. it is part of data retrieved from the server through a REST call. I do not keep a reference to it once I build the UI. I will keep it in mind for other uses. – adaj21 Dec 29 '13 at 19:25
  • If you don't keep the reference so use a property to save the contents for each index, like a.__some = someCollection[i]. you could save what you want in a javascript object, it's dynamic. – Sayris Dec 29 '13 at 21:28
1

Interesting approach below. I also found the following which works exactly as I want so I thought to share here:

function attachSomeMethodClickHandler(a, value){
   function functionX(){callSomeMethod(value);};
   a.addEventListener('click', functionX);
}
 :
 :

for(var i in someCollection){
    var a = document.createElement('a');
    attachSomeMethodClickHandler(a, someCollection[i]);
    :
}
adaj21
  • 543
  • 3
  • 11
  • 25
0

Don't use inline bindings but instead try using event delegation:

Bind an event to the anchors parent and check the target once it's clicked,

this way, you're not limited to the amount of elements which are created,

and don't have to bind the event again if you'll create new ones later on.

Then pass the anchors index instead of a parameter. 

var dataSource = ["dog", "cat", "horse"];
var container = document.getElementById("container");

function index(el) {
  var parent = el.parentNode;
  for(i = 0;i < parent.childNodes.length;i++) {
    if(parent.childNodes[i] == el) {
        return i;   
    }
  }
}

container.addEventListener("click", function (e) {
  e.preventDefault();
  var idx = index(e.target);
  alert("Index: " + idx + " value: " + dataSource[idx]);
});

for (i = 0; i < dataSource.length; i++) {
  var data = dataSource[i];
  var a = document.createElement("a");
  var text = document.createTextNode(data);
  a.href = "http://someurl.com?id=" + data;
  a.appendChild(text);
  container.appendChild(a);
}

Here's the fiddle: http://jsfiddle.net/RZw8e/2/

silicakes
  • 6,364
  • 3
  • 28
  • 39
  • Tanks Mike86. As per my comment to Sayris, I do not keep a reference to the collection. – adaj21 Dec 29 '13 at 19:30
  • You're welcome, notice that you could really simplify things using jQuery, and unless you have a reason for not utilizing it inside your code - i strongly recommend you to try it. – silicakes Dec 29 '13 at 20:45