Javascript for loop and alert

Question

I am looping through a list of links. I can correctly get the title attribute, and want it displayed onclick. When the page is loaded and when I click on a link, all of the link titles are alerted one by one. What am I doing wrong?

function prepareShowElement () {
var nav = document.getElementById('nav');
var links = nav.getElementsByTagName('a');
for (var i = 0; i < links.length; i++) {
    links[i].onclick = alert(links[i].title);
    }
}

This is your first language isn't it? You would do so much better to learn programming in another language. Javascript will teach you nothing that another language won't, but it's got the power to shoot you in the foot every time you pick it up. I know programming experts who do very wrong things in Javascript because they don't understand it. — jcolebrand, Jul 18 '11 at 06:51
@jcolebrand: Yeah, those other languages will do him much good when he need to show alerts in a website.... — Thilo, Jul 18 '11 at 06:55
I think it is a good idea to start learning programming with javascript. Professional programmers usually have lots of trouble with javascript, because they try to apply other highlevel technics found in languages like c++ to javascript, and completely miss the point. — Ibu, Jul 18 '11 at 07:00
@Ibu: I feel JS needs more "high level" things, but am not sure they are related to why the language can be tricky? @Squadrons: javascript used to be a good language to learn, relative to the other options. I might recommend python now, but they have the same issues. There's one gotcha with both these languages: you have to declare new functions whenever you want to create snapshots of the stack (what programmers call "closures"). In javascript, I would actually recommend using https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Array#Iteration_methods rather than for-loops. — ninjagecko, Jul 18 '11 at 07:06

score 7 · Accepted Answer · answered Jul 18 '11 at 06:48

7

What you were doing was actually running the alert function. enclosing the whole thing in an anonymous function will only run it when it is clicked

for (var i = 0; i < links.length; i++) {
    links[i].onclick = function () {
        alert(this.title);
       }
    }

answered Jul 18 '11 at 06:48

Ibu

42,752
13
76
103

This won't work because neither the value of links or i will still be valid when the alert runs. See PaulPRO's answer for the right way to do this. – jfriend00 Jul 18 '11 at 06:51
+1 for getting the first correct and simple version. It is amazing how many get this wrong (though you missed it the first time). – jfriend00 Jul 18 '11 at 06:57
@Ibu: You need to add `var self = this;` outside the onclick handler and use `self.title` inside. – ThiefMaster Jul 18 '11 at 07:27
3

@ThiefMaster - `var self = this` is not required or useful in this case. 'this' will be set by the event handler to point to the element that received the click before the event handler is called. There are other cases where that piece of code is useful, but not here. You can see this in action in this jsFiddle: http://jsfiddle.net/jfriend00/W24nN/ – jfriend00 Jul 18 '11 at 07:47
Here's a similar example using an unordered list: [http://jsfiddle.net/cLSnX/](http://jsfiddle.net/cLSnX/) – popedotninja May 22 '14 at 22:01

score 2 · Answer 2 · answered Jul 18 '11 at 06:48

2

You are assigning the onclick to the return value of alert(links[i].title); which doesn't make any sense, since onclick is supposed to be a function.

What you want instead is somethig like onclick = function(){ alert('Hi'); };

But

Since you are using a variable i in that loop you need to create a local copy of it onclick = function(){ alert(links[i].title); }; would just use the outer scope i and all your links would alert the same message.

To fix this you need to write a function that localizes i and returns a new function specific to each link's own onclick:

onclick = (function(i){ return function(e){ alert(links[i].title); }; })(i);

Final result:

function prepareShowElement () {
var nav = document.getElementById('nav');
var links = nav.getElementsByTagName('a');
for (var i = 0; i < links.length; i++) {
    links[i].onclick = (function(i){ return function(e){ alert(links[i].title); }; })(i);
    }
}

answered Jul 18 '11 at 06:48

Paul

139,544
27
275
264

+1 if you explain why the extra closure is necessary to capture i. – Thilo Jul 18 '11 at 06:50
3

Would probably be better to just get the title from `this.title` rather than use the more complicated closure. – jfriend00 Jul 18 '11 at 06:52
1

That's true actually! links[i] is the element that the click is being triggered on, so the closure isn't even needed if you don't use `i` at all. I like Ibu's solution better then. – Paul Jul 18 '11 at 06:55
1

+1 because your solution was the only one in this whole thread that would work the first time it was posted and you took the time to explain what that OP was doing wrong. Ibu had to correct his and when he did, he corrected it to the simpler version. – jfriend00 Jul 18 '11 at 07:00

score 2 · Answer 3 · edited Jul 18 '11 at 07:13

2

You can use jquery. To display title of the link on click.

$("#nav a").click(function() {
    var title = $(this).attr('title');
    alert(title);
});

edited Jul 18 '11 at 07:13

ninjagecko

88,546
24
137
145

answered Jul 18 '11 at 06:50

Ajinkya

22,324
33
110
161

1

@jfriendOO: I understood that he wants to display title of the link on click. In hurry I typed `href` instead of `title`. @ninjagecko: Thanks man. – Ajinkya Jul 18 '11 at 07:00
1

@ninjagecko or Ajinkya can you edit it one more time to be `$('#nav a')` since in the OP's post he was using #nav as the context for searching for anchors. – Paul Jul 18 '11 at 07:12
@PaulPRO:Thanks for suggestion now onward will read the complete question first :) @ninjagecko: Thanks again :) – Ajinkya Jul 18 '11 at 07:23
@Ajinkya and @ninjagecko: Part of my response was because jQuery isn't what the OP asked for either. IMO, the value in answering this question is to explain what they got wrong in their code and with their approach. Yes, this will work in a jQuery environment (now that it's been corrected) - but that isn't what the OP asked for. – jfriend00 Jul 18 '11 at 08:06
1

@jfriendOO: I really appreciate your concern. But it is always good if we can suggest any better way.I also come to know about Jquery in same way, I asked JS question and someone gave Jquery answer. Now I am in love with Jquery :) – Ajinkya Jul 18 '11 at 08:39

score 1 · Answer 4 · edited May 23 '17 at 12:32

links.forEach(function(link) {
    link.onclick = function(event) {
        alert(link.title);
    };
}

Also note that your original solution suffered from this problem: JavaScript closure inside loops – simple practical example

By passing in our iteration variable into a closure, we get to keep it. If we wrote the above using a for-loop, it would look like this:

// machinery needed to get the same effect as above
for (var i = 0; i < links.length; i++) {
    (function(link){
        link.onclick = function(event) {
            alert(link.title);
        }
    })(links[i])
}

or

// machinery needed to get the same effect as above (version 2)
for (var i = 0; i < links.length; i++) {
    (function(i){
        links[i].onclick = function(event) {
            alert(links[i].title);
        }
    })(i)
}

score -2 · Answer 5 · answered Jul 18 '11 at 06:51

-2

You need change .onclick for a eventlistener same:

function prepareShowElement () {
  var nav = document.getElementById('nav');
  var links = nav.getElementsByTagName('a');
  for (var i = 0; i < links.length; i++) {
    links[i].addEventListener('click',function() { 
        alert(links[i].title);
    },false);
  }
}

answered Jul 18 '11 at 06:51

jbalde

62
1

It amazing how many people misunderstand how the values of links and i are no longer valid when the onclick handler is called and thus this will not work. And, there is no requirement to change to addEventListener (which won't work in IE either). – jfriend00 Jul 18 '11 at 06:56

Javascript for loop and alert

5 Answers5

Linked

Related