How can I refactor this JavaScript code to avoid making functions in a loop?

Question

I wrote the following code for a project that I'm working on:

var clicky_tracking = [
  ['related-searches', 'Related Searches'],
  ['related-stories', 'Related Stories'],
  ['more-videos', 'More Videos'],
  ['web-headlines', 'Publication']
];

for (var x = 0, length_x = clicky_tracking.length; x < length_x; x++) {
  links = document.getElementById(clicky_tracking[x][0])
                  .getElementsByTagName('a');
  for (var y = 0, length_y = links.length; y < length_y; y++) {
    links[y].onclick = (function(name, url) {
      return function() {
        clicky.log(url, name, 'outbound');
      };
    }(clicky_tracking[x][1], links[y].href));
  }
}

What I'm trying to do is:

• define a two-dimensional array, with each instance the inner arrays containing two elements: an id attribute value (e.g., "related-searches") and a corresponding description (e.g., "Related Searches");
• for each of the inner arrays, find the element in the document with the corresponding id attribute, and then gather a collection of all <a> elements (hyperlinks) within it;
• loop through that collection and attach an onclick handler to each hyperlink, which should call clicky.log, passing in as parameters the description that corresponds to the id (e.g., "Related Searches" for the id "related-searches") and the value of the href attribute for the <a> element that was clicked.

Hopefully that wasn't thoroughly confusing! The code may be more self-explanatory than that.

I believe that what I've implemented here is a closure, but JSLint complains:

http://img.skitch.com/20100526-k1trfr6tpj64iamm8r4jf5rbru.png

So, my questions are:

• How can I refactor this code to make JSLint agreeable? Or, better yet, is there a best-practices way to do this that I'm missing, regardless of what JSLint thinks?
• Should I rely on event delegation instead? That is, attaching onclick event handlers to the document elements with the id attributes in my arrays, and then looking at event.target? I've done that once before and understand the theory, but I'm very hazy on the details, and would appreciate some guidance on what that would look like - assuming this is a viable approach.

Thanks very much for any help!

Answer 1

Delegation is a better approach. JSLint complains because you are creating a new function each time within the loop. However, instead of setting up a single event on the document to listen for all click events, I would rather setup a handler on all individual root elements that have id's assigned to them. A single handler can be reused for all these elements.

function logClick(event) {
    event = event || window.event;
    var link = event.target || event.srcElement;

    if(link.nodeName.toLowerCase() !== "a") {
        return;
    }

    var name = clicky_tracking[this.id];
    clicky.log(link.href, name, 'outbound');
}

Register the above handler with each root element.

for(var id in clicky_tracking) {
    var root = document.getElementById(id);
    root.onclick = logClick;
}

Also to avoid searching through the array, I've changed clicky_tracking from array to an object for easier keyed access.

var clicky_tracking = {
  'related-searches': 'Related Searches',
  'related-stories': 'Related Stories',
  'more-videos': 'More Videos',
  'web-headlines': 'Publication'
};