HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

JavaScript Tic-Tac-Toe web app

Submitted by: @import:stackexchange-codereview··
0
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

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.

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.