patternjavascriptMinor
JavaScript Tic-Tac-Toe web app
Viewed 0 times
appjavascripttoetictacweb
Problem
I am teaching myself JavaScript and made a little Tic-Tac-Toe web app. I'm not asking anyone to read through it all, but to just tell me what I need or should not do in the future.
`var boxes = document.querySelectorAll("td");
var boxtext = document.querySelectorAll("td span");
var player = document.querySelector("#player");
var playerTurnDisplay = document.querySelector("#playerTurnDisplay");
var resetButton = document.querySelector("#resetButton");
var isXTurn = true;
var boxesLeftToClick = 9;
var gameOver = false;
function init() {
addClickListeners(boxes);
}
function addClickListeners(arr) {
for (var i = 0; i 0) {
player.textContent = (isXTurn) ? "X" : "O";
playerTurnDisplay.classList.toggle("xTurn");
playerTurnDisplay.classList.toggle("yTurn");
} else {
playerTurnDisplay.textContent = "Draw";
playerTurnDisplay.classList.toggle(playerTurnDisplay.classList);
playerTurnDisplay.classList.add("gameOver");
}
}
// if someone wins get rid of the background color and display the text of whoever won
function setWinnerDisplay(box, orientation) {
playerTurnDisplay.classList.remove("xTurn");
playerTurnDisplay.classList.remove("yTurn");
playerTurnDisplay.classList.add("gameOver");
playerTurnDisplay.textContent = box.textContent + " wins " + orientation;
gameOver = true;
}
// adds the "line" through the winning boxes
function crossOut(box1, box2, box3) {
box1.classList.add("crossOut");
box2.classList.add("crossOut");
box3.classList.add("crossOut");
}
// all the things that need to happend when a player wins will happen if you call this function
function win(box1, box2, box3, orientation) {
setWinnerDisplay(box1, orientation);
crossOut(box1, box2, box3);
}
// returns true if the boxes are all X's or O's false if they are empty or not the same
function isTheSame(box1, box2, box3) {
if (box1.textContent === box2.textContent && box1.textContent === box3.textCont
`var boxes = document.querySelectorAll("td");
var boxtext = document.querySelectorAll("td span");
var player = document.querySelector("#player");
var playerTurnDisplay = document.querySelector("#playerTurnDisplay");
var resetButton = document.querySelector("#resetButton");
var isXTurn = true;
var boxesLeftToClick = 9;
var gameOver = false;
function init() {
addClickListeners(boxes);
}
function addClickListeners(arr) {
for (var i = 0; i 0) {
player.textContent = (isXTurn) ? "X" : "O";
playerTurnDisplay.classList.toggle("xTurn");
playerTurnDisplay.classList.toggle("yTurn");
} else {
playerTurnDisplay.textContent = "Draw";
playerTurnDisplay.classList.toggle(playerTurnDisplay.classList);
playerTurnDisplay.classList.add("gameOver");
}
}
// if someone wins get rid of the background color and display the text of whoever won
function setWinnerDisplay(box, orientation) {
playerTurnDisplay.classList.remove("xTurn");
playerTurnDisplay.classList.remove("yTurn");
playerTurnDisplay.classList.add("gameOver");
playerTurnDisplay.textContent = box.textContent + " wins " + orientation;
gameOver = true;
}
// adds the "line" through the winning boxes
function crossOut(box1, box2, box3) {
box1.classList.add("crossOut");
box2.classList.add("crossOut");
box3.classList.add("crossOut");
}
// all the things that need to happend when a player wins will happen if you call this function
function win(box1, box2, box3, orientation) {
setWinnerDisplay(box1, orientation);
crossOut(box1, box2, box3);
}
// returns true if the boxes are all X's or O's false if they are empty or not the same
function isTheSame(box1, box2, box3) {
if (box1.textContent === box2.textContent && box1.textContent === box3.textCont
Solution
1.
Consider using ES2015. Most of its features are there to make your life easier. Modern browsers support > 95% of it and for the rest there's transpilers/polyfills (babel).
2.
should be just
This is general advice that is valid for other programming language. You can do the same for the isTheSame function.
I also feel like the condition should be
3.
The checkWinner can be rewritten in a less repetitive fashion:
Why is an array better than that long if-else chain? Because you can potentially store these combinations in a separate file or even generate them. If you decide to have 4x4 tic-tac-toe you're going to get tired of typing them out explicitly.
It's generally a good idea to store data-heavy/table-looking code as data.
4.
Consider using
Consider using ES2015. Most of its features are there to make your life easier. Modern browsers support > 95% of it and for the rest there's transpilers/polyfills (babel).
2.
function isEmpty(box) {
if (box.textContent !== "X" && box.textContent !== "O")
return true;
else
return false;
}should be just
function isEmpty(box) {
return box.textContent !== "X" && box.textContent !== "O"
}This is general advice that is valid for other programming language. You can do the same for the isTheSame function.
I also feel like the condition should be
box.textContent === ''; this is more in line with the isEmpty name.3.
The checkWinner can be rewritten in a less repetitive fashion:
const winningCombination = [
{ triple: [0, 1, 2], name: 'horizontally' },
{ triple: [3, 4, 5], name: 'horizontally' },
// etc
].find(({ triple }) => isTheSame(...triple))
if (winningCombination) {
const { triple, name } = winningCombination
win(...triple, name)
}Why is an array better than that long if-else chain? Because you can potentially store these combinations in a separate file or even generate them. If you decide to have 4x4 tic-tac-toe you're going to get tired of typing them out explicitly.
It's generally a good idea to store data-heavy/table-looking code as data.
4.
Consider using
forEach/for ... of instead of plain for loops. In both of your cases you don't care how you're iterating arr/boxes. Let forEach/for ... of concern themselves with HOW the iteration is done.Code Snippets
function isEmpty(box) {
if (box.textContent !== "X" && box.textContent !== "O")
return true;
else
return false;
}function isEmpty(box) {
return box.textContent !== "X" && box.textContent !== "O"
}const winningCombination = [
{ triple: [0, 1, 2], name: 'horizontally' },
{ triple: [3, 4, 5], name: 'horizontally' },
// etc
].find(({ triple }) => isTheSame(...triple))
if (winningCombination) {
const { triple, name } = winningCombination
win(...triple, name)
}Context
StackExchange Code Review Q#149362, answer score: 3
Revisions (0)
No revisions yet.