Jump to content

JavaScript closures


spence911

Recommended Posts

Whats up guys. I've created two links in an html document. The problem is the JavaScript below gives me a value of undefined after running the for loop. I know this has something to do with closures, which I don't have much experience with so I can't figure out why the value of links is undefined.

 

JavaScript:

window.onload = function getLinks(){
		
		var links = document.links;
		for(var i = 0, count = links.length; i < count; i++){
			links[i].onclick = function(){
				alert(links[i].href);
				return false;
			};
		}
		
};

html:

<body>
<p><a href="linkOne.html" id="link" target="linkOne">Link B</a></p>
<p><a href="linkTwo.html" id="link" target="linkTwo">Link A</a></p>
<script src="js/handleLinks.js"></script>
</body>
</html>
Link to comment
https://forums.phpfreaks.com/topic/288137-javascript-closures/
Share on other sites

The closure will be using whatever the value of links.href is at the time of execution. Which is long after that for loop has ended and i is past the end of links.

 

1. Don't assign closures directly to on* properties. Use the proper event registration system for the browser, which is addEventListener() for most browsers and attachEvent() for older IE.

2. Don't return false from the handler. Use the event object to stopPropagation and/or preventDefault. Again, IE is the village idiot: event.stopPropagation() for most browsers versus event.cancelBubble=true for older IE, and event.preventDefault() versus event.returnValue=false.

 

2a. To the point, the most common way around the value issue is another closure.

links[i].addEventListener("click", (function(link) {
	return function(e) {
		alert(link.href);
		e.preventDefault();
	};
})(link[i]));
The outer function is executed immediately and returns the actual event handler. That inner function uses link which was defined when the function executed.

 

2b. However there's a simpler way of doing it.

links[i].addEventListener("click", function(e) {
	alert(this.href);
	e.preventDefault();
});
Don't even need links at all.

 

2c. And if you started using a Javascript framework, like jQuery, it's even easier:

$(function() { // replaces window.onload/window.addEventListener(...)
	$("a[href]").click(function(e) {
		alert(this.href);
		e.preventDefault();
	});
});
Forget links entirely.
Link to comment
https://forums.phpfreaks.com/topic/288137-javascript-closures/#findComment-1477736
Share on other sites

Thanks for the workaround. So if I understand correctly, the value of links is undefined because once the for loop has finished executing, whatever new value the for loop has created for var links is thrown out and replaced by the original value which is var links = document.links; correct?

Link to comment
https://forums.phpfreaks.com/topic/288137-javascript-closures/#findComment-1477818
Share on other sites

Thanks for the workaround. So if I understand correctly, the value of links is undefined because once the for loop has finished executing, whatever new value the for loop has created for var links is thrown out and replaced by the original value which is var links = document.links; correct?

 

Here's another way of explaining the problem that may make sense. That loop is defining a function to run when the links are clicked. If you strip out just that function you end up with this

 

function(){
                alert(links[i].href);
                return false;
            };

 

links is not defined when the function is called by the onclick event. It is not inserting the value of links[i].href on each iteration of the loop. But, that's just the wrong way to do this anyway. For that to work you have to dynamically create a unique function for each hyperlink. Instead, just create one function that the links pass an event handler to so the function can reference the href value - as requinix showed.

Link to comment
https://forums.phpfreaks.com/topic/288137-javascript-closures/#findComment-1477824
Share on other sites

Archived

This topic is now archived and is closed to further replies.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.