Jump to content

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

  • Solution

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

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • 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.