I'm trying to build a tic-tac-toe game and the result that is displayed ("winner", "tie") works fine unless there is only one box left on the board and the player wins with that last move. Then depending on the combination the result can be correct ("Player1 WINS") or wrong ("TIE").
Here is the code that displays the winner. Is there something wrong with it?
const boxes = document.querySelectorAll('.box');
let winner = document.querySelector('.winner');
const playerFactory = (name, symbol) => {
return {name, symbol};
};
const player1 = playerFactory('Player1', 'X');
const player2 = playerFactory('Player2', 'O');
const gameBoard = (() => {
const board = [];
const addMark = () => {
boxes.forEach(function(box) {
box.addEventListener('click', () =>{
if(box.textContent === '') {// Check if the box is empty
if (gameBoard.board[0] == null || (gameBoard.board.length > 1 && gameBoard.board[gameBoard.board.length - 1] == player2.symbol)){
gameBoard.board.push(player1.symbol);
} else gameBoard.board.push(player2.symbol)
box.textContent = gameBoard.board[gameBoard.board.length - 1];
}
})
})
}
return {board, addMark};
})();
gameBoard.addMark();
function displayWinner(){
const arr =[];
const winCombinations = [[0, 1, 2],
[0, 3, 6],
[1, 4, 7],
[2, 5, 8],
[3, 4, 5],
[6, 7, 8],
[2, 4, 6],
[0, 4, 8]];
boxes.forEach(box => arr.push(box.textContent));
let p1Array = arr.map((item, index) => {
return item === "X" ? index : null;
}).filter(index => index !== null);
let p2Array = arr.map((item, index) => {
return item === "O" ? index : null;
}).filter(index => index !== null);
winCombinations.forEach((elem, index) => {
if(winCombinations[index].every(item => p1Array.includes(item))){
return winner.textContent = `${player1.name} WINS`;
} else if(winCombinations[index].every(item => p2Array.includes(item))){
return winner.textContent = `${player2.name} WINS`;
} else if(arr.every(item => item !== '')) return winner.textContent = 'TIE';
});
}
Thanks in advance!
Your forEach loop has return statements, but those do not terminate the loop. The forEach callbacks will all be called, and if a subsequent one assigns something to winner.textContent then that will override any previous value assigned to it.
We can reproduce that with this snippet:
// The initialisation that is needed to run the displayWinner code
const player1 = {name: "X"};
const player2 = {name: "O"};
const s = "XXX"+
"XOO" +
"OOX";
const boxes = document.querySelectorAll("div span");
boxes.forEach((box, i) => box.textContent = s[i].trim());
// Call it
displayWinner();
function displayWinner(){
const arr =[];
const winCombinations = [[0, 1, 2],
[0, 3, 6],
[1, 4, 7],
[2, 5, 8],
[3, 4, 5],
[6, 7, 8],
[2, 4, 6],
[0, 4, 8]];
boxes.forEach(box => arr.push(box.textContent));
let p1Array = arr.map((item, index) => {
return item === "X" ? index : null;
}).filter(index => index !== null);
let p2Array = arr.map((item, index) => {
return item === "O" ? index : null;
}).filter(index => index !== null);
winCombinations.forEach((elem, index) => {
if(winCombinations[index].every(item => p1Array.includes(item))){
return winner.textContent = `${player1.name} WINS`;
} else if(winCombinations[index].every(item => p2Array.includes(item))){
return winner.textContent = `${player2.name} WINS`;
} else if(arr.every(item => item !== '')) return winner.textContent = 'TIE';
});
}outcome: <span id="winner"></span><p>
Game board:
<div>
<span></span><span></span><span></span><br>
<span></span><span></span><span></span><br>
<span></span><span></span><span></span><br>
</div>Use a for loop instead, so that the return statement ends the loop. Secondly, there is no need to check in every iteration if it is a draw. This can happen after the loop.
Correction:
// The initialisation that is needed to run the displayWinner code
const player1 = {name: "X"};
const player2 = {name: "O"};
const s = "XXX"+
"XOO" +
"OOX";
const boxes = document.querySelectorAll("div span");
boxes.forEach((box, i) => box.textContent = s[i].trim());
// Call it
displayWinner();
function displayWinner(){
const arr =[];
const winCombinations = [[0, 1, 2],
[0, 3, 6],
[1, 4, 7],
[2, 5, 8],
[3, 4, 5],
[6, 7, 8],
[2, 4, 6],
[0, 4, 8]];
boxes.forEach(box => arr.push(box.textContent));
let p1Array = arr.map((item, index) => {
return item === "X" ? index : null;
}).filter(index => index !== null);
let p2Array = arr.map((item, index) => {
return item === "O" ? index : null;
}).filter(index => index !== null);
for (let combi of winCombinations) {
if(combi.every(item => p1Array.includes(item))){
return winner.textContent = `${player1.name} WINS`;
} else if(combi.every(item => p2Array.includes(item))){
return winner.textContent = `${player2.name} WINS`;
}
}
if(arr.every(item => item !== '')) winner.textContent = 'TIE';
}outcome: <span id="winner"></span><p>
Game board:
<div>
<span></span><span></span><span></span><br>
<span></span><span></span><span></span><br>
<span></span><span></span><span></span><br>
</div>Some other improvements:
forEach+push should better be written with Array.fromp1Array or p2Array. Instead use the indices of winning combinations to read directly from arr and see if that is "XXX" or "OOO".Boolean as callback for every.So:
// The initialisation that is needed to run the displayWinner code
const player1 = {name: "X"};
const player2 = {name: "O"};
const s = "XXX"+
"XOO" +
"OOX";
const boxes = document.querySelectorAll("div span");
boxes.forEach((box, i) => box.textContent = s[i].trim());
// Call it
displayWinner();
function displayWinner(){
const winCombinations = [[0, 1, 2],
[0, 3, 6],
[1, 4, 7],
[2, 5, 8],
[3, 4, 5],
[6, 7, 8],
[2, 4, 6],
[0, 4, 8]];
const arr = Array.from(boxes, box => box.textContent);
for (const combi of winCombinations) {
const three = combi.map(item => arr[item]).join("");
if (three == "XXX") {
return winner.textContent = `${player1.name} WINS`;
} else if (three == "OOO") {
return winner.textContent = `${player2.name} WINS`;
}
}
if (arr.every(Boolean)) winner.textContent = 'TIE';
}outcome: <span id="winner"></span><p>
Game board:
<div>
<span></span><span></span><span></span><br>
<span></span><span></span><span></span><br>
<span></span><span></span><span></span><br>
</div>