0

I am currently programming a checkers game for a high school project. This code is a part of what I have which has an issue):

function CreateBoard() {
    var board = document.createElement("table");
    board.cellSpacing = 0;
    for (var i = 0; i < 8; i++) {
        var tr1 = document.createElement("tr");
        for (var j = 0; j < 8; j++) {
            var td1 = document.createElement("td");
            td1.setAttribute("id", "td" + i + j);
            td1.addEventListener("click", function () { Check(i, j); });
            if (i % 2 == 0) {
                if (j % 2 == 0)
                    td1.style.backgroundColor = "beige";
                else
                    td1.style.backgroundColor = "black";
            }
            else {
                if (j % 2 == 0)
                    td1.style.backgroundColor = "black";
                else
                    td1.style.backgroundColor = "beige";
            }
            tr1.appendChild(td1);
        }
        board.appendChild(tr1);
    }
    document.body.appendChild(board);
} 

function Check(i, j) {
    alert("Check i: " + i);
    alert("Check j: " + j);
}

Now, here's the problem: When I run my code using HTML, I see the board perfectly. However, when I click one of the cells testing my event listeners, I get that i and j are always equal to 8. My teacher helped me figure out that the problem arises from the fact that the event listeners are loaded after the loop runs, so that i and j always get their last value, which is 8. She hasn't provided me with a solution yet. Does anyone know how to solve this problem? Is anyone willing to help me with this? It will help me enormously.

Heretic Monkey
  • 11,687
  • 7
  • 53
  • 122
  • It would make it a lot easier to help if you included the HTML on which this code runs. You can use Stack Snippets (icon looks like `<>` in the editor toolbar) to make it even easier by allowing it to run here on Stack Overflow. – Heretic Monkey Apr 17 '21 at 22:57
  • 1
    You should also read the answers to https://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example :). – Heretic Monkey Apr 17 '21 at 22:58
  • There's nothing special there, just linking the JS file and running the 'CreateBoard()` function. –  Apr 17 '21 at 22:59
  • Does this answer your question? [JavaScript closure inside loops – simple practical example](https://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) – Heretic Monkey Apr 17 '21 at 23:00
  • I have been referenced to this answer, but I couldn't find any answers for my case, because in my case the problem arises from the event listener loading after the loop ends. –  Apr 17 '21 at 23:00
  • Right, that's precisely what that question, and its answers, are talking about. `i` and `j` are referencing the final values of the loops. You need to use a closure so that it references the values they are at the time the loop is registering the event handler. – Heretic Monkey Apr 17 '21 at 23:02

3 Answers3

1

For starters, your structure is bad because you are appending the TD tags before the TR has been appended!

You cannot attach children until all parents are in place!

Append the TR immediately like this... (I've moved the board-append also!)

function CreateBoard() {
    var board = document.createElement("table");
    board.cellSpacing = 0;
    document.body.appendChild(board);      //  <--- the same with the board!

    for (var i = 0; i < 8; i++) {
        var tr1 = document.createElement("tr");
        board.appendChild(tr1);      // <--- MOVE IT UP HERE
            
        for (var j = 0; j < 8; j++) {
            var td1 = document.createElement("td");
            td1.setAttribute("id", "td" + i + j);
            td1.addEventListener("click", function () { Check(i, j); });

            if (i % 2 == 0) {
                if (j % 2 == 0)
                    td1.style.backgroundColor = "beige";
                else
                    td1.style.backgroundColor = "black";
            }
            else {
                if (j % 2 == 0)
                    td1.style.backgroundColor = "black";
                else
                    td1.style.backgroundColor = "beige";
            }
             
            tr1.appendChild(td1);
        }        
    }
} 
MB_
  • 1,667
  • 1
  • 6
  • 14
TodayCoder
  • 11
  • 2
0

Indeed as far as I understand the problem comes from the event listener callback function. In JS there is this concept called Closures, often also named Lambda Expressions or Arrow-Functions (they look like this (parameter) => {code}). Closures are basically small functions, but with the functionality that they capture the scope in which they had been created.

In your case you have to just replace your regular function in this line:

td1.addEventListener("click", function () { Check(i, j); });

with this closure:

td1.addEventListener("click", () => { Check(i, j); }); //
David B.
  • 695
  • 5
  • 10
  • Thank you, but I tried your suggestion and it didn't succeed to solve the problem. I copy-pasted your code line instead of the previous one, but it didn't work. –  Apr 17 '21 at 23:05
0

This should solve the issue:

td1.addEventListener("click", ((a, b) => { return function () { Check(a, b); }})(i, j));

This is typical problem with Javascript closures and loops. You should investigate it and read more about that. Here's nice starting point.

Vejsil Hrustić
  • 179
  • 1
  • 7
  • Thank you so much! this one seems to work. I haven't learned about closures in school so I'll investigate and read more about them. Thank you again! I truly appreciate it. –  Apr 17 '21 at 23:07
  • oh yeah. great solution @Vejsil Hrustić. Creating an arrow function, which returns a function, and call the closure right away, so that the resulting event callback is immediately available. like that :) – David B. Apr 17 '21 at 23:13