javascript closure not working

perlmonk · #1 06-25-2008, 10:00 AM

Hi All

I tried the following code

Code:

  window.onload = function() 
     {
       var list = ['abc', 'xyz'] ; // id's from links
       for(var i = 0; i < list.length; i++ )
        {
          var id = list[i] ;
          document.getElementById(list[i]).onclick = function() { alert("link with id=" + id) }
        }
     }

Also

Code:

document.getElementById(list[i]).onclick = function() { alert("link with id=" + list[i]) }

doesn't work

So, when I click on the link with id 'abc' I get the alert showing "link with id=xyz"
For some reason the closure only memorizes the last values! (and also doesn't remember list)

Any ideas ?

thnx in advance
LuCa

**Kor** · #2 06-25-2008, 10:11 AM

Why not simply return the element's id on using the self reference this?

Code:

for(var i = 0; i < list.length; i++ )
        {
          document.getElementById(list[i]).onclick = function() { alert("link with id=" + this.id) }
        }

On the other hand, if you need to use the increment [i], you must know that it is an independent variable and it will take, inside the closure, its last value after the loop is finished. A solution is to stick that value of the increment as a custom property of the element:

Code:

for(var i = 0; i < list.length; i++ )
{
var d=document.getElementById(list[i]);
d.indexV=i;
d.onclick = function() { alert("link with id=" + this.indexV) }
}

perlmonk · #3 06-25-2008, 10:20 AM

thnx, so this means that javascript doesn't do closures, you just need to stick all the variables in the object itself!

**Kor** · #4 06-25-2008, 10:26 AM

You need to stick only the variables which depends of the increment in a loop. As I said, in a closure, the increment will always take its last value of the loop, because the loop code ends before the closure has the chance to be active. I think this is true in all the languages which uses closures.

perlmonk · #5 06-26-2008, 02:54 AM

Here is a perl example which shows that you can do what I want:

#! /usr/bin/perl -l

my @array ;
{
my @vals = qw(a b c d e) ;
foreach(0..4)
{
my $inp = $vals[$_] ;
my $i = $_ ;
$array[$_] = sub { print "i = " . $i . " and val = " . $inp . " from ".$vals[$i] } ;
}
}

$array[0]() ;
$array[3]() ;

prints:
i=0 and val = a from a
i=3 and val = d from d

Orc Scorcher · #6 06-26-2008, 03:22 AM

That is because you're using block-scoped variables in your Perl example but function-scoped variables in your JavaScript example. Block-scoped variables were added to JavaScript in version 1.7, try the following example in Fx3:

Code:

<script type="text/javascript;version=1.8">

function test() {
	var chars = "abcde"
	var funcs = []
	for (let _ = 0; _ < 5; ++_) {
		let i = _
		let val = chars[i]
		funcs[i] = function () "i = " + i + " and val = " + val + " from " + chars[i]
	}
	return funcs
}

var f = test()
document.write(f[0](), "<br>", f[3]())

</script>

romo · #7 06-26-2008, 03:47 AM

You simply don't use closure, use this code instead:

Code:

  window.onload = function() 
     {
       var list = ['abc', 'xyz'] ; // id's from links
       for(var i = 0; i < list.length; i++ ) new function()
        {
          var id = list[i] ;
          document.getElementById(list[i]).onclick = function() { alert("link with id=" + id) }
        }
     }

**Kor** · #8 06-26-2008, 03:52 AM

Oh, I see. Well, yes, in javascript you may do that this way:

Code:

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
<html>
<head>
<title>untitled</title>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<script type="text/javascript">
var array;
var vals=['a','b','c','d','e'];
var inp;
for(var i=0;i<=4;i++){
window['array']=function(i){
inp=vals[i];
var d=document.getElementById('mydiv');
var inTxt=d.innerHTML+'i ='+i+' and val = '+inp+' from '+vals[i]+'<br>';
d.innerHTML=inTxt;
}
}
onload=function(){
array(0);
array(3);
}
</script>
</head>
<body>
<div id="mydiv"></div>
</body>
</html>

But here you are dealing with global variables (global scope) not with local variables (function scope). In your first example your variables were local, thus the closure cold not keep them.

Logic Ali · #9 06-26-2008, 05:50 AM

Quote:

Originally Posted by romo

You simply don't use closure, use this code instead:

Code:

  window.onload = function() 
     {
       var list = ['abc', 'xyz'] ; // id's from links
       for(var i = 0; i < list.length; i++ ) new function()
        {
          var id = list ;
[i]          document.getElementById(list).onclick = function() { alert("link with id=" + id) }
       }
    }
[i]

That code is forming closures. The new operator just has the effect of executing the function, instead of calling it like this:

Code:

 
for(var i = 0; i < list.length; i++ ) 
(function() 
{
 var id = list[i] ;
 document.getElementById(list[i]).onclick = function() { alert("link with id=" + id) }
})();

forty2 · #10 06-26-2008, 07:25 AM

Or like this:

Code:

window.onload = function()
{
	var o_links = document.getElementsByTagName('a');
	for (var i = 0, len = o_links.length; i < len; i++)
	{
		o_links[i].onclick = function(sender_num)
		{
			return function()
			{
				alert(sender_num);
			}
		}(i);
	}
}

**Kor** · #11 06-26-2008, 07:59 AM

There are a lot of ways to skin a cat

perlmonk · #12 06-28-2008, 10:03 AM

thnx a lot!!!

cooper.semantic · #13 07-19-2008, 07:15 PM

Hey guys,
I'm having an issue figuring this out. I have read through this forum but still am running into a spike. If someone could help I would be greatly thankful.

Here is the javascript:
function init() {
var first = getElementsByClass(document, "first", "*");
for (var i=0; i < first.length; i++) {
var list = first[i]
list.onclick = (function() {
return function() {
var last = getElementsByClass(document, "last", "*")[0];
toggle(last);
};
})();
}
}

function toggle(ob) {
if (ob.style.display == "block") {
ob.style.display = "none";
}
else { ob.style.display = "block"; }
}

Here is the HTML:
<ul>

<li class="first">First</li>
<li style="display: none;" class="last">Last</li>

<li class="first ">First</li>
<li style="display: none;" class="last ">Last</li>

<li class="first ">First</li>
<li style="display: none;" class="last ">Last</li>
</ul>

**Declan1991** · #14 07-19-2008, 07:43 PM

You should start your own thread in future.
If they are every second turn like that, JavaScript is capable of figuring that out, you don't need all the classes.

Code:

function init() {
	var li = document.getElementById("list").getElementsByTagName("li");
	for (var i = 0; i < li.length; i+=2) {
		li[i].onclick = (function() {
			var last = li[i+1], shown = false;
			last.style.display="none";
			return function() {
				last.style.display = shown?"none":"block";
				shown = !shown;
			};
		})();
	}
}

Code:

<ul id="list">

<li>First</li>
<li>Last</li>

<li>First</li>
<li>Last</li>

<li>First</li>
<li>Last</li>
</ul>

The fix to your code would be

Code:

function init() {
var first = getElementsByClass(document, "first", "*");
var lasts = getElementsByClass(document, "last", "*");
for (var i=0; i < first.length; i++) {
var list = first[i]
list.onclick = (function() {
var last = lasts[i];
return function() {
toggle(last);
};
})();
}
}

cooper.semantic · #15 07-19-2008, 08:05 PM

I see where I went wrong. I did not want to start my own thread with the same topic. I figured since this was an already established thread with my same problem it would be just fine. I thank you though for helping me out.

Thread Tools	Search this Thread
Show Printable Version Email this Page	Search this Thread: Advanced Search
Display Modes	Rate This Thread
Linear Mode Switch to Hybrid Mode Switch to Threaded Mode	Rate This Thread:

Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)