Javascript - onClick event passing same parameter to function

Question

I am iterating over elements in a container and adding onClick to every element. I wish to execute a method which accepts an id as parameter when I click on an element. The id is unique to each element.

Below is my code:

$(document).ready(function() 
{
    var conElement = document.getElementById("my-container");
    if (conElement)
    {
        var bookElements = conElement.getElementsByClassName("book-elements");

        for (var element = bookElements.length; element--; )
        {
            var bookElement = bookElements[element];
            var bookId = bookElement.getAttribute('book-id');

            alert ('Book id is ' + bookId);
            bookElement.onclick = function() {showBookSummary(bookId)};
        }
    }

});

function showBookSummary(bookId)
{
    alert ('Book id inside showBookSummary is ' + bookId);
}

Issue: Same book id is passed everytime I click on any element. Suppose I have 3 elements and id for 1st element is "123", 2nd element is "456" and 3rd element is "789". Whenever I click on any of the 3 elements, it always passes id "123" as bookId to showBookSummary function.

Am I missing anything in here?

Answer 1

PROBLEM

Your issue is caused because bookId is a regular JS variable that retains its value. Instead of saving the bookId of that book, the bookId is what it is after running the code.

The code execution would be as follows:

bookId = 789 (id of first book because you are starting at top and decrementing)
bookId = 456 (second element)
bookId = 123 (third element)

And then bookId is still 123, because you are still using the bookId variable. Therefore, anytime you call:

showBookSummary(bookId);

bookId is still 123.

---

SOLUTION

Instead, I think you should change the line:

bookElement.onclick = function() {showBookSummary(bookId)};

to:

bookElement.onclick = (function(t){return function(){showBookSummary(t);};})(bookId);

edit: onclick handler credits to @RobG from comments. Yes, I realize now that putting it as a string wouldn't work. This does something similar by substituting bookId for t and essentially storing the correct bookId instead of using the variable that changes during execution.

EDIT: Possibly easier solution:

According to the other SO answer (marked duplicate), it also seems that the relatively new let keyword could also help in this context. It creates a variable local to the scope of the loop, not the function. So if you just change the declaration of bookId:

var bookId = ...

to let:

let bookId = ...

This should work as well. I haven't tried it yet, but it looks promising.

---

On a side note, if you are using jQuery, you can replace the dreaded:

document.getElementById("id")
document.getElementsByClassName("class")
for(var element = bookElements.length; element--; )
getAttribute("attr")
element.onclick = function()

with jQuery's built-in shorthands:

$("#id")
$(".class")
elements.each(
attr("attr")
elements.click(function()