Tic-tac-toe with JS

Question

I am a beginner and was hoping for some honest feedback on my js code. I struggled quite a bit on this, I did ask gpt to explain some concepts which I find it’s good for but wrote it all myself.

// variable to store array of players 
const players = ['X', 'O'];
// declaring currentplayer with array index 
let currentPlayer = players[0];
let xScore = 0;
let oScore = 0;
// store cell class into cellElements 
const cellElements = document.querySelectorAll('.cell');
const resetBtn = document.querySelector('.reset');
const resultElem = document.querySelector('#results');
// array of possible win combinations 
const winningMoves = [
 [0, 1, 2],
 [3, 4, 5],
 [6, 7, 8],
 [0, 3, 6],
 [1, 4, 7],
 [2, 5, 8],
 [0, 4, 8],
 [2, 4, 6] 
];
// for of loop to loop through each cell and add event listener 
for (const cellElem of cellElements) {
 cellElem.addEventListener('click', handleClick);
}
// function to execute click in each cell 
function handleClick(event) {
 const clickedCellElem = event.target;
 // if else condition checking if cell is empty then inputting current play and switching between X and O 
 if (clickedCellElem.textContent === '') {
 clickedCellElem.textContent = currentPlayer;
 // calling checkWin function after each move 
 if (checkWin()) {
 resultElem.textContent = (`${currentPlayer} wins!`);
 // disable cell if win
 disableCell();
 } else if (checkDraw()) {
 disableCell();
 resultElem.textContent = (`It's a draw!`);
 }
 // if else condition to switch between the current player array index 
 if (currentPlayer === players[0]) {
 currentPlayer = players[1];
 } else {
 currentPlayer = players[0];
 }
 }
}
// check for a draw after no winning combinations 
function checkDraw(){
 for (const cellElem of cellElements)
 if (cellElem.textContent === '') {
 return false
 }
 return true
}
// function to execute when player wins by having 3 cells in a row with X or O
function checkWin() {
 // loops through each element in winning combinations 
 for (const winningMove of winningMoves) {
 // extracts individual elements into three separate variables to store the index of winnings combinations array 
 const [cellIndex1, cellIndex2, cellIndex3] = winningMove;
 // DOM grabbing html cell ID content of cells which corresponds to each winning combination index in array
 const cell1Content = document.getElementById(`cell-${cellIndex1}`).textContent;
 const cell2Content = document.getElementById(`cell-${cellIndex2}`).textContent;
 const cell3Content = document.getElementById(`cell-${cellIndex3}`).textContent;
 
 // condition to compare each 3 cells are the same to declare winner 
 if (cell1Content !== '' && cell1Content === cell2Content && cell2Content === cell3Content) {
 //increments player score 
 if (currentPlayer === players[0]) {
 xScore++;
 // displays the score getting DOM id element 
 document.getElementById("X-score").textContent = `[X] ${xScore}`;
 } else {
 oScore++;
 document.getElementById("O-score").textContent = `[O] ${oScore}`;
 }
 return true;
 }
 }
 return false;
}
// function to reset the game
function resetGame() {
 //loop through cell elements 
 for (const cellElem of cellElements) {
 //reset the cells to empty string 
 cellElem.textContent = '';
 // add event listener again after cell disabled 
 cellElem.addEventListener('click', handleClick);
 }
 // reset result message 
 resultElem.textContent = '';
}
// Attach the resetGame function to resetBtn
resetBtn.addEventListener('click', resetGame);
//function to disbable the cells after win 
function disableCell() {
 for (const cellElem of cellElements) {
 cellElem.removeEventListener('click', handleClick);
 }
}

body {
 display: flex;
 flex-direction: column;
 align-items: center;
 justify-content: center;
 margin: 0;
 font-family: Arial, sans-serif;
 background-color: #000;
 margin-top: 7rem;
}
h1 {
 font-size: 2em;
 margin-bottom: 30px;
 /* color: yellow; */
 color: #FFF01F;
 font-family: "Retro Gaming";
 border: 2px solid #88be19;
 padding: 10px;
 border-image: url("data:image/svg+xml;charset=utf-8,%3Csvg width='100' height='100' viewBox='0 0 100 100' fill='none' xmlns='http://www.w3.org/2000/svg'%3E %3Cstyle%3Epath%7Banimation:stroke 5s infinite linear%3B%7D%40keyframes stroke%7Bto%7Bstroke-dashoffset:776%3B%7D%7D%3C/style%3E%3ClinearGradient id='g' x1='0%25' y1='0%25' x2='0%25' y2='100%25'%3E%3Cstop offset='0%25' stop-color='%232d3561' /%3E%3Cstop offset='25%25' stop-color='%23c05c7e' /%3E%3Cstop offset='50%25' stop-color='%23f3826f' /%3E%3Cstop offset='100%25' stop-color='%23ffb961' /%3E%3C/linearGradient%3E %3Cpath d='M1.5 1.5 l97 0l0 97l-97 0 l0 -97' stroke-linecap='square' stroke='url(%23g)' stroke-width='3' stroke-dasharray='388'/%3E %3C/svg%3E") 1;
}
header {
 display: flex;
 align-items: center;
 justify-content: space-between;
}
h2 {
 font-size: 1.5em;
 margin-bottom: 30px;
 color: #39FF14;
 font-family: "Retro Gaming";
}
.board {
 display: grid;
 grid-template-columns: 1fr 1fr 1fr;
 gap: 2px;
 margin-bottom: 2rem;
 max-width: 100%;
}
.cell {
 width: 110px;
 height: 110px;
 border: 1px solid #FFF01F;
 box-sizing: border-box;
 display: flex;
 align-items: center;
 justify-content: center;
 font-size: 4.5rem;
 cursor: pointer;
 font-family: "Retro Gaming";
 color: blue;
}
.cell:hover {
 background-color: #1a1b1c;
 opacity: 1.0;
}
h3 {
 border: 2px solid #88be19;
 color: #39FF14;
 padding: 13px;
 text-align: center;
 font-family: "Retro Gaming";
 font-size: 20px;
 margin-bottom: 8px;
}
.player-score {
 padding: 5px;
 padding-top: 20px;
 margin: 5px;
 font-family: "Retro Gaming";
 color: #39FF14;
}
.reset {
 margin-bottom: 15rem;
 color: #f55b1d;
 font-family: "Retro Gaming";
 background-color: #000;
 border-radius: 2rem;
 cursor: pointer;
 font-size: 18px;
}
.reset:hover {
 transform: scale(1.2);
}

<!DOCTYPE html>
<html lang="en">
<head>
 <meta charset="UTF-8">
 <meta name="viewport" content="width=device-width, initial-scale=1.0">
 <link rel="stylesheet" href="style.css">
 <link href="https://db.onlinewebfonts.com/c/4c19fc875e7ba1e6831129de3ab5ac0b?family=Retro+Gaming" rel="stylesheet">
 <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/animate.css/4.1.1/animate.min.css" />
 <title>Document</title>
</head>
<body>
 <h1 class="animate__animated animate__bounce">Tic-Tac-Toe</h1>
 <header class="container">
 <div class="player-score" id="X-score">[X] 0</div>
 <h3>Scoreboard</h3>
 <div class="player-score" id="O-score">[O] 0</div>
 </header>
 <h2 id="results"></h2>
 <div class="board">
 <div class="cell" id="cell-0"></div>
 <div class="cell" id="cell-1"></div>
 <div class="cell" id="cell-2"></div>
 <div class="cell" id="cell-3"></div>
 <div class="cell" id="cell-4"></div>
 <div class="cell" id="cell-5"></div>
 <div class="cell" id="cell-6"></div>
 <div class="cell" id="cell-7"></div>
 <div class="cell" id="cell-8"></div>
 </div>
<div class="button-container">
 <button class="reset">New Game</button>
</div>
 <script src="game.js"></script>
</body>
</html>

Answer 1

There are only a few things I would change with your JS

if/else vs switch

if (checkWin()) {
 resultElem.textContent = (`${currentPlayer} wins!`);
 // disable cell if win
 disableCell();
} else if (checkDraw()) {
 disableCell();
 resultElem.textContent = (`It's a draw!`);
}

This part is repetitive and low-performing. If you have 2 if statements then you are nearly always better off with a switch statement. In this specific case, I would change the code completely:

switch (true) {
 case checkWin():
 endGame(`${currentPlayer} wins!`);
 break;
 case checkDraw():
 endGame(`It's a draw!`);
 break;
 default:
 break; 
}
function endGame(text) {
 disableCell();
 resultElem.textContent = text;
}

This will perform faster for most browsers and you remove the repetitive code as in both cases you write a result message and disable all cells.

if/else vs ternary vs modulus

You have another if/else statement block that could be simplified as it is repetitive again.

if (currentPlayer === players[0]) {
 currentPlayer = players[1];
} else {
 currentPlayer = players[0];
}

The most obvious change would be the usage of a ternary conditional operator:

currentPlayer = players[currentPlayer === players[0] ? 1 : 0];

You use a ternary to check the condition within the assignment with following syntax:

condition ? true : false

const players = ['X', 'O'];
let currentPlayer = players[0];
function changePlayer() {
 currentPlayer = players[currentPlayer === players[0] ? 1 : 0];
 
 console.clear();
 console.log(currentPlayer);
}

<button onclick="changePlayer()">change player</button>

Another method would be the usage of a modulus here. However you would need to change the logic here to output the index number with currentPlayer:

const players = ['X', 'O'];
let currentPlayer = 0;
function changePlayer() {
 currentPlayer = (currentPlayer + 1) % players.length;
 
 console.clear();
 console.log(players[currentPlayer]);
}

<button onclick="changePlayer()">change player</button>

return true/false vs ! (not) statement

You have a line with an if statement to return the negated condition.

if (cellElem.textContent === '') {
 return false
}
return true

In this case, you check a condition and want to return true if it is return false. The same vice-versa.
It can be simplified;

return !cellElem.textContent === '';

A condition will already return true or false So you simply negate the condition with a not or !.

You could also do a double negation on textContent.length to check if the element is empty or not:

function checkTextContent(cellElem) {
 return !!cellElem.textContent.length;
}
console.log(checkTextContent(document.querySelector('#element-1')));
console.log(checkTextContent(document.querySelector('#element-2')));

<div id="element-1">Test</div>
<div id="element-2"></div>

---

Overall you have a good understanding of how to use if/else conditions but lack the knowledge about other more suitable methods and when to use them.

• Toby Speight: Instead of using %, toggling between two players could also be done using 1 - currentPlayer
• tacoshy: @TobySpeight I totally agree. This is coming from more abstract cases where you potentially have more than 2 players. Or instances that you want to infinitely iterate through an array. I hope that he just remembers when he has to iterate though a larger array that you can also use the modulus.