Search code examples
javascriptfunctionvariablestic-tac-toescope-chain

Change global variable from event function


I am trying to learn Javascript.

I watched some Javascript course on Youtube and am trying to create my first project.

Project should be simple tic-tac-toe game.

Functionality: First click on box should fill with "X" Second click on another box should fill with "Y" Third click on another box should fill with "X" again and so on until all boxes will be filled with character.

there is my codes HTML

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Document</title>
    <link rel="stylesheet" href="style.css">
</head>
<body>
<div class="main">
<table>
    <tr>
        <td class="b1-1"></td>
        <td class="b1-2"></td>
        <td class="b1-3"></td>
    </tr>
    <tr>
        <td class="b2-1"></td>
        <td class="b2-2"></td>
        <td class="b2-3"></td>
    </tr>
    <tr>
        <td class="b3-1"></td>
        <td class="b3-2"></td>
        <td class="b3-3"></td>
    </tr>
</table>
</div>
<script src="script.js" type="text/javascript"></script>
</body>
</html>

CSS

.main {
    padding: 100px 0;
    width: 360px;
    margin: 0 auto;
}

table, tbody {
    margin: 0;
    padding: 0;
    width: 360px;
    height: 360px;
}

tr {
    width:360px;
    height: 120px;
    margin: 0;
    padding: 0;
}

td {
    text-align: center;
    width:120px;
    height: 120px;
    border: 1px solid #333;
    margin: 0;
    padding: 0;
    font-size: 50px;
}

Javascript

var action = document.querySelectorAll('td');
var gameState = 0;
for (var i = 0; i <= action.length - 1; i++) {
    getClassName = "." + action[i].classList.value;
        if (gameState === 0) {
            document.querySelector(getClassName).addEventListener("click", chasviX(i));
        } else {
            document.querySelector(getClassName).addEventListener("click", chasviO(i));
        }
}

function chasviX(cord) {
        document.querySelector("." + action[cord].classList.value).addEventListener("click", event => {
            document.querySelector("." + action[cord].classList.value).textContent = "X";
            gameState = 1;
        });
};

function chasviO(cord) {
        document.querySelector("." + action[cord].classList.value).addEventListener("click", event => {
            document.querySelector("." + action[cord].classList.value).textContent = "O";
            gameState = 0;
        });
};

There is project link also - https://jsfiddle.net/qwy2k571/

At the moment each box is filling with "X". I know i didn't understand closures and scope chains exactly but please give me an correct code to understand it by example.

Thanks in advance and best regards.


Solution

  • In your first loop : for (var i = 0; i <= action.length - 1; i++) { ... } you are adding events listeners to each cells. Since the variable gameState is initialized to 0, the condition if (gameState === 0) { ... } will always be true within that loop.

    Instead of checking the status of the game during the initialization, just add an event listener as you did, to each cells.

    To make sure you pass the correct parameter to the event, you need to wrap the callback within function() { chasvi(param); }, and the whole body of the loop inside another anonymous function, setting another variable to the coordinate i :

    for (var i = 0; i <= action.length - 1; i++) {
        (function(){
              let coor = i;
              let getClassName = "." + action[i].classList.value;
              document.querySelector(getClassName).addEventListener("click", function() { chasvi(coor); });
        }());
    }
    

    Note that I changed the name of the function to chasvi because you can manage the case X or O in that function.


    In each of the functions chasviX and chasviO you are again adding an event listener. This is not good, because on each clicks, you'll add one more event.

    Before clicking, there's already an event.

    After clicking once, there are 2 events. Another click, and there's 3 events, and so on...

    I suggest you to change thoses functions to 1 function that handles both cases :

    function chasvi(cord)
    {
        let TextToDisplay;
    
        if (gameState === 0)
        {
            TextToDisplay = "X";
            gameState = 1;
        }
        else
        {
            TextToDisplay = "O";
            gameState = 0;
        }
        document.querySelector("." + action[cord].classList.value).textContent = TextToDisplay;
    }
    

    Since there's only 2 states for gameState, you can use a boolean value. You can initialize it to var gameState = false; I randomly choose false for the X.

    And then, the function can be simplified to :

    function chasvi(cord)
    {
        let TextToDisplay;
    
        if (gameState === false)
        {
            TextToDisplay = "X";
        }
        else
        {
            TextToDisplay = "O";
        }
        gameState = !gameState; // This swaps the state true/false
        document.querySelector("." + action[cord].classList.value).textContent = TextToDisplay;
    }
    

    This kind of notation :

    let TextToDisplay;
    
    if (gameState === false)
    {
        TextToDisplay = "X";
    }
    else
    {
        TextToDisplay = "O";
    }
    

    Can be simplified using ternary expressions :

    let TextToDisplay = gameState ? "O" : "X"; // This means if gameState is true, then "O" is returned, else "X"
    

    Final code can looks like this :

    var action = document.querySelectorAll('td');
        var gameState = false;
        for (var i = 0; i <= action.length - 1; i++) {
            (function(){
              let coor = i;
              getClassName = "." + action[i].classList.value;
              document.querySelector(getClassName).addEventListener("click", function() { chasvi(coor); });
             }());
        }
        
        function chasvi(cord) {
            let TextToDisplay = gameState ? "O" : "X";
            gameState = !gameState;
            document.querySelector("." + action[cord].classList.value).textContent = TextToDisplay;
        }
    .main {
            padding: 100px 0;
            width: 360px;
            margin: 0 auto;
        }
        
        table, tbody {
            margin: 0;
            padding: 0;
            width: 360px;
            height: 360px;
        }
        
        tr {
            width:360px;
            height: 120px;
            margin: 0;
            padding: 0;
        }
        
        td {
            text-align: center;
            width:120px;
            height: 120px;
            border: 1px solid #333;
            margin: 0;
            padding: 0;
            font-size: 50px;
        }
    <!DOCTYPE html>
        <html lang="en">
        <head>
            <meta charset="UTF-8">
            <meta name="viewport" content="width=device-width, initial-scale=1.0">
            <title>Document</title>
            <link rel="stylesheet" href="style.css">
        </head>
        <body>
        <div class="main">
        <table>
            <tr>
                <td class="b1-1"></td>
                <td class="b1-2"></td>
                <td class="b1-3"></td>
            </tr>
            <tr>
                <td class="b2-1"></td>
                <td class="b2-2"></td>
                <td class="b2-3"></td>
            </tr>
            <tr>
                <td class="b3-1"></td>
                <td class="b3-2"></td>
                <td class="b3-3"></td>
            </tr>
        </table>
        </div>
        <script src="script.js" type="text/javascript"></script>
        </body>
        </html>