how to create a function with a static parameter

concerning unobtrusive js

[Skier88]

I am writing code that takes an element's class and uses it to add event handlers to that element. For example:

<input type='checkbox' class='checkbox toggle-div123'/>

becomes equivalent to

<input type='checkbox' class='checkbox toggle-div123' onchange='toggle(document.getElementById("div123"),this.checked);'/>

I have most of the code working, but I can't figure out how to create a static variable for the first parameter of toggle(). That is to say, when I set the onchange handler to function() {toggle(matches[m],this.checked);}, that is setting the first parameter to point to matches[m], while what I want it to do is always be the value matches[m] was when I created the function (since I am looping through checkboxes, matches[m] changes).

Here's my code:

function setToggles()
{
var tmp;
var inputs=document.getElementsByTagName('input');
var regex=/(?:^|\s)toggle-(\S+)/g;
for(var i=0;i<inputs.length;i++)
{
var e=inputs[i];
if(e.type=='checkbox'&&e.className.indexOf('toggle-')!=-1)
{
var matches=e.className.match(regex);
for(var m in matches)
e.onchange=function() {toggle(matches[m].substring(8),this.checked);};
}
}
}

I do have an idea which I have used successfully in experimental websites, which is to assign a new attribute to the element. eg, e.setAttribute('toggle',matches[m]), and e.onchange=function() {toggle(this.getAttribute('toggle'),this.checked);}. Or do a similar approach with a javascript array that isn't part of the page. But this seems rather clumsy - this isn't the "correct" solution, is it?

[Fotiman]

What you need is a "closure".

Try this:

e.onchange = function(el){
  return function() {
    toggle(el, this.checked);
  }
}(matches[m].substring(8));

In other words, onchange gets assigned the results of the call to an anonymous function which takes 1 parameter and returns a function. The value of matches[m].substring(8) is passed in to the anonymous function as parameter el. When the function assigned to the onchange handler executes, it references the value el instead of the matches[m] array (which now points to the last item in the matches array).

Closers can be tricky to get a grasp on at first. Don't be afraid to ask questions. :)

[Skier88]

Great, thank you - it works flawlessly. It makes perfect sense, I guess I just wasn't thinking javascript.

[daveVk]

Alternative approach

<input type='checkbox' class='checkbox toggle-div123' onchange='toggle(this);'/>

Inside toggle 'div123' can be extracted from className.

[Skier88]

Ok, it turns out in only works in good browsers. I'm getting an error in IE (8, I haven't tested other versions): "Object doesn't support this property or method / line 13 char 1" (position adjusted to example). In IE the first insertion works fine (toggles back and forth), but the rest don't do anything. Here's the new code:

function setToggles()
{
var tmp;
var inputs=document.getElementsByTagName('input');
var regex=/(?:^|\s)toggle-(\S+)/g;
for(var i=0;i<inputs.length;i++)
{
var e=inputs[i];
if(e.type=='checkbox'&&e.className.indexOf('toggle-')!=-1)
{
var matches=e.className.match(regex);
for(var m in matches)
e.onchange=function(t) {return function() {toggle(t,this.checked);}}(matches[m].substring(8));
}
}
}

Also, I have a quick question regarding the regex - is there any way to save the callbacks? As you can see I wrote the regex with the intention of using $1 for the id instead of creating a substring of the whole match, since the former seems more efficient.

Thanks for the reply daveVk, but my goal is to entirely separate the javascript and html.

[daveVk]

for(var m in matches)
e.onchange=...
}

There seems little point setting onchange multiple times ?

The javascript and html can be separated, regardless of when to toggle is discovered.

function setToggles()
{
var inputs=document.getElementsByTagName('input');
for(var i=0;i<inputs.length;i++)
{
var e=inputs[ i ];
if(e.type=='checkbox'&&e.className.indexOf('toggle-')!=-1) {
e.onchange=doToggle
}
}

Where doToggle can unpack the classname, and handle multiple toggles if that was intent of prior code.

This also avoids creating a new function for each checkbox.

[Fotiman]

Ok, looking closer, there are a couple of issues.

1. for(var m in matches)
Apparently IE treats matches like an object and iterates through all of the object properties instead of just the array of matches. Changing it to an array syntax seems to fix it.

2. The other big issue is that you're attaching to the onchange event of the checkbox button. But that event won't fire until the checkbox loses focus. You probably want to use the onclick event instead.

3. Setting the event handler multiple times will simply replace the previous handler (vs. calling multiple event handlers). So for example, if your checkbox looked like this:
<input type='checkbox' class='checkbox toggle-div123 toggle-div456'/>
and you were expecting it to toggle both divs, only the last one defined would toggle. Instead of using event handlers, a better option would be to use event Listeners. You can any number of listeners. Alternatively, a more efficient approach might be to modify your toggle function to take an array of elements, generate the array from the matches values, and assign that to your event handler.

Here is a reworked example, fully commented, which will hopefully get you going the right direction. Note, I added a DEBUG variable that might help during development. My example includes toggling multiple elements (note, my toggle function simply alerts a message to show that the handler is working, it does not actually toggle the elements display), and includes an error case (trying to toggle an element that could not be found in the DOM). :)

<!DOCTYPE html>
<head>
<title>Test</title>
</head>
<body>
<div id="div123">123</div>
<div id="div456">456</div>
<div id="div789">789</div>
<input type='checkbox' class='checkbox toggle-div123 toggle-div789'>
<input type='checkbox' class='checkbox toggle-div456 toggle-div000'>
<input type='checkbox' class='checkbox toggle-div000'>
<script type="text/javascript">
function toggle(els, b) {
  var i, n;
  for (i = 0, n = els.length; i < n; i++) {
    alert('toggle(' + els[i].id + ', ' + b + ')');
  }
}
function setToggles() {
  var DEBUG = true, // set to true for debug alerts (during development)
    el,     // will hold reference to input element
    i,     // array index variable
    inputs = document.getElementsByTagName('input'),
    matches,  // will hold array of toggleRegEx matches
    n,     // will hold the length of array
    toggleRegEx = /(?:^|\s)toggle-(\S+)/g;
  // iterate through all input elements
  for (i = 0, n = inputs.length; i < n; i++) {
    el = inputs[i];
    // check to see if this is a checkbox that matches our 'toggle' class
    if (el.type == 'checkbox' && el.className.indexOf('toggle-') != -1) {
      // get the array of toggle matches
      matches = el.className.match(toggleRegEx);
      if (matches.length > 0) {
        // create the onclick event handler using array of matches
        el.onclick = (function (matches) {
          var elToggle,  // will hold reference to dom element
            els = [],  // will hold array of elements
            j,     // array index variable
            k;     // will hold the length of array
          for (j = 0, k = matches.length; j < k; j++) {
            if (DEBUG) {
              alert('adding ' + matches[j].substring(8));
            }
            elToggle = document.getElementById(matches[j].substring(8));
            if (elToggle) {
              els.push(elToggle);
            }
            else if (DEBUG) {
              // Attempted to add toggle handler for unknown el
              alert('Element not found: [' + 
                  matches[j].substring(8) + ']');
            }
          }
          if (els.length > 0) {
            return function() {
              toggle(els, this.checked);
            };
          }
          else {
            if (DEBUG) {
              alert('Assigning empty function to event handler');
            }
            return function () {};
          }
        })(matches);
      }
    }
  }
}
setToggles();
</script>
</body>
</html>

[Skier88]

So that's how you format code ... the site really gives no indication of it that I've seen.

Sorry I haven't replied ... I thought I had just after daveVk, but apparently that either me or my computer messed up - or the server with these updates. And I didn't get an email for the second reply. Anyway, thank you for the replies - I have done a bit of reworking myself and will post a full response after I've thoroughly read your posts and finished my code (assuming I now can).

[Skier88]

daveVk, sorry, I didn't realize your inline js was just shorthand. And yes, that loop was there because I had been meaning to get to appending the function instead of replacing it - see below.

As for your approach, it looks like a good idea, but it increases the work done on every click, and requires either a function to interface with the toggle() function or for the toggle() function to be more purpose-specific. On the other hand, jslint also told me not to create functions in a loop. I think that I can tolerate that though, since I know that the loop is not creating an excessive number of functions.

Fotiman, thank you for your code - it's clear enough to be in a textbook. However, I don't know if passing an array is the best approach. Since the checkbox may already have an onclick function the code needs to know how to append a function anyway, so appending each toggle reduces the amount of code (something I like perhaps too much). Besides, the vast majority of the toggling checkboxes will only toggle one element, so overall efficiency will likely remain the same. So I modified your code to append each toggle function to the previous onclick function.

Here's the new code ... I know I butchered the jslint-perfect formatting, but the code is more readable to me with nearly minimal spacing. I put in an equivalent dummy toggle function for the sake of clarity. The script now works everywhere I've tested it - thanks for your help!

function toggle(el,b) {
 alert('toggle('+el.id+','+b+')');
}

function setToggles() {
  var el,matches;
  var inputs=document.getElementsByTagName('input');
  var regex=/(?:^|\s)toggle-(\S+)/g;
  for(var i=0;i<inputs.length;i++) {
   el=inputs[i];
   if(el.type=='checkbox'&&el.className.indexOf('toggle-')!=-1) {
  matches=el.className.match(regex);
  for(var j=0;j<matches.length;j++)
  el.onclick=function(old,target) {
   if(!target) return old;
   if(old) return function() {old.call(this,arguments); toggle(target,this.checked);};
   return function() {toggle(target,this.checked);};
  }(el.onclick,document.getElementById(matches[j].substring(8)));
 }
 }
}

[Fotiman]

Since the checkbox may already have an onclick function the code needs to know how to append a function anyway,

This is where you could benefit from using Event Listeners instead of Event Handlers (you can have any number of Listeners without having to know if there is an onclick function defined already). In addition, if you ever add code after your code that also sets the onclick handler, your code might be replaced (just another reason to use Listeners instead).

But it sounds like you've got a solution you're happy with. :)

[Skier88]

Sorry, I was going to edit in a little more (including make the formatting nicer) but I lost my connection. Anyway, yes, I read a little about event listeners, but it sounded like they are a huge pain compared to handlers. One source said there is no universal support; I imagine that is outdated, but I would still need browser-specific code.

Also, thanks for the onclick tip. Another thing lost in my earlier post ... I thought IE was just screwing with me for a good 10 minutes. But between that and your discovery about the for loop I've got it covered now, like you said. Thanks again.

[daveVk]

it increases the work done on every click

Agree, the trade off is doing 1 unit of work onclick vs say 100 units of similar work (for 100 checkboxs) on page load. I doubt if any performance difference would be noticed.

Yes it would require a function to interface with the toggle() function, with code more or less cut from setToggles. This function replaces the say 100 anon functions otherwise generated.

I would also recommend event Listeners.

[Fotiman]

There is universal support for event listeners in all browsers you'd need to worry about. However, the implementation can be different per browser. A good alternative is to use one of the popular frameworks because they've taken care of all the cross browser issues, and are generally pretty lightweight. You might check out jQuery or YUI. Or you can find plenty of addEvent methods online.

[Skier88]

daveVk, ok, you make a good point. I am just hesitant because I feel like clicking on something to trigger an event shouldn't need to figure out which event to trigger every time. But as you point out, if I were to have considerably more checkboxes the start time could become an issue. I'll give it a try.

And as for event listeners, ok, thanks for the suggestion. I'll try that out too. I was just so proud of my handler :p

I would prefer not to use a framework at this point, just to keep things as light as possible. I know they are exceptionally light considering their functionality, but as I don't need all that functionality a vanilla solution will be lighter.

[Fotiman]

In that case, you might define your own addEvent function:

function addEvent(el, type, fn) {
  if (window.addEventListener) {
    // This browser supports the standard defined addEventListener, so use it
    el.addEventListener(type, fn, false);
  }
  else if (window.attachEvent) {
    // This browser supports IE's attachEvent method
    el.attachEvent('on' + type, fn);
  }
  else {
    // fallback to event handler for REALLY old browsers
    el['on' + type] = fn;
  }
}

That might handle the majority of cases. Typical usage then would be:

addEvent(el, 'click', function () {
  // do something
});

[astupidname]

Quick tip, Internet Explorer's attachEvent method botches the 'this' object within functions that are passed to it to be set as event handlers. So, while this does work fine as long as you are not attempting to access the 'this' object within fn:

 else if (window.attachEvent) {
  // This browser supports IE's attachEvent method
  el.attachEvent('on' + type, fn);
 }

it is better written utilizing the trickery of javascript's call() method [developer.mozilla.org] as below technique will maintain the appropriate 'this' object within handlers passed to it:

 else if (window.attachEvent) {
  // This browser supports IE's attachEvent method
  //actually, is almost definitely IE, have not heard of others mimicking it's stupidity?
  el.attachEvent('on' + type, function () { return fn.call(el, window.event)});
 }

So, say for example you set a listener where 'el' is a button or link or such:

addEvent(el, 'click', function () {
  alert(this);
});

In Internet Explorer, using the previous method would cause the alert to incorrectly be "[object Window]", whereas the latter method would result in "[object HTMLButtonElement]" if el were a button.
The latter method, with it's ability to properly access the 'this' object, is also preferred as you need not maintain backreferences to the element it's self when defining the handler (a potential memory leak), or getting a reference to the element from the event parameter, as the reference is already maintained within the handler as 'this' and makes coding a bit easier in long run.

[Fotiman]

Thanks for pointing that out astupidname. That was what I was referring to when I wrote "That might handle the majority of cases", but I didn't have time to elaborate (and I was hoping someone else might chime in). Thanks. :)

[Skier88]

Great, thanks for the code again. I updated addEvent with astupidname's addition and it works great. I suppose this method would be preferable if I ever wanted to remove the listener.

This thread has been really great - not only did I get an answer, but also a good deal of code quality tips, which I really appreciate. Thanks again Fotiman, daveVk, and astupidname.