patternjavascriptMinor
JavaScript Tic Tac Toe game challenge
Viewed 0 times
challengejavascripttoetictacgame
Problem
I just completed the Tic Tac Toe challenge at freecodecamp. I am looking for input on how I could shorten the code. For the click events such as
Codepen
background-color: gray;
}
table, td{
border: 2px solid black;
margin:0 auto;
}
.tablecon {
display:none;
}
#table {
background-color:
s1.onclick, I feel that it could have been shortened in some way, maybe using the this object. I feel as though that my detectWin function has too much code as well.Codepen
//Game function
function tictac() {
"use strict";
var tableI = document.getElementById("table");
var X = document.querySelector(".X");
X.addEventListener("click", function() {
player.gameSym = "X";
AI.gameSym = "O";
start();
});
var O = document.querySelector(".O");
O.addEventListener("click", function() {
player.gameSym = "O";
AI.gameSym = "X";
start();
});
var cells = document.querySelectorAll("td");
var para = document.querySelectorAll(".first");
var table = document.querySelector(".tablecon");
var gameEnd = false;
var s1 = document.getElementById("s1");
var s2 = document.getElementById("s2");
var s3 = document.getElementById("s3");
var s4 = document.getElementById("s4");
var s5 = document.getElementById("s5");
var s6 = document.getElementById("s6");
var s7 = document.getElementById("s7");
var s8 = document.getElementById("s8");
var s9 = document.getElementById("s9");
var winner = document.querySelector(".winner");
var td = document.querySelectorAll("td");
//create player and computer objects
var player = {
gameSym: "",
hasTurn: false,
hasWon: false
};
var AI = {
gameSym: "",
hasTurn: false,
hasWon: false
};
//Contains game control functions
function start() {
for (var i = 0; i
body { background-color: gray;
}
table, td{
border: 2px solid black;
margin:0 auto;
}
.tablecon {
display:none;
}
#table {
background-color:
Solution
This game plays OK--if you can use a mouse. If you try to use a keyboard or voice, it is unplayable.
First, your prompt:
You cannot "click" the
Now, your
You could put a
Now you have one block of code that handles all of your squares.
As for your
You should also pull your win/lose strings into constants so you only have one value to update if you ever change them.
First, your prompt:
A game of Tic Tac Toe
Do you want to be X or O?
X
O
You cannot "click" the
X or O text with your keyboard since the browser just sees it as a text element with an event handler. These should be button elements; you can style them so they look the same, but now the browser can make the buttons available to the keyboard, so you can TAB to them. Then, you handle the click event in your JS, just the same as you are now.Now, your
gameControl function repeats itself something fierce. For example:s1.onclick = function() {
if ($.trim($("#s1").html()) === '') {
s1.innerHTML = player.gameSym;
changeTurn();
}
};
s2.onclick = function() {
if ($.trim($("#s2").html()) === '') {
s2.innerHTML = player.gameSym;
changeTurn();
}
};You could put a
class on each square, then do something like this (I'm using jQuery here since I'm familiar with it, but you can do it without jQuery too).$('.square').click(function(e) {
var square = $(e.target); // get clicked element
if ($.trim(square.html()) === '') {
square.innerHTML = player.gameSym;
changeTurn();
}
});Now you have one block of code that handles all of your squares.
As for your
detectWin function, I'd load the data into a 2d array. Then you can loop over your data, as in:for (let row = 0; row < 3; row++) {
if (arr[row][0] !== '' && // make sure it has an entered value
arr[row][0] === arr[row][1] && // compare square 1 and 2 in the row
arr[row][0] === arr[row][2]) // compare square 1 and 3 in the row
{
// horizontal win--handle text for X/O win
if (arr[row][0] === 'x') {}
else if (arr[row][0] === 'o') {}
break;
}
}
for (let col = 0; col < 3; col++) {
if (arr[0][col] !== '' && // make sure it has an entered value
arr[0][col] === arr[1][col] && // compare square 1 and 2 in the column
arr[0][col] === arr[2][col]) // compare square 1 and 3 in the column
{
// vertical win--handle text for X/O win
if (arr[0][col] === 'x') {}
else if (arr[0][col] === 'o') {}
break;
}
}
if (arr[0][col] !== '' &&
arr[0][0] === arr[1][1] &&
arr[0][0] === arr[2][2] ||
arr[0][2] !== '' &&
arr[0][2] === arr[1][1] &&
arr[0][2] === arr[2][0])
{
// diagonal win--handle text for X/O win
if (arr[row][0] === 'x') {}
else if (arr[row][0] === 'o') {}
}You should also pull your win/lose strings into constants so you only have one value to update if you ever change them.
Code Snippets
<div class="prompt">
<h1>A game of Tic Tac Toe</h2>
<p class="quest first">Do you want to be X or O?</p>
<div class="xocon">
<p class="X first">X</p>
<p class="O first">O</p>
</div>
</div>s1.onclick = function() {
if ($.trim($("#s1").html()) === '') {
s1.innerHTML = player.gameSym;
changeTurn();
}
};
s2.onclick = function() {
if ($.trim($("#s2").html()) === '') {
s2.innerHTML = player.gameSym;
changeTurn();
}
};$('.square').click(function(e) {
var square = $(e.target); // get clicked element
if ($.trim(square.html()) === '') {
square.innerHTML = player.gameSym;
changeTurn();
}
});for (let row = 0; row < 3; row++) {
if (arr[row][0] !== '' && // make sure it has an entered value
arr[row][0] === arr[row][1] && // compare square 1 and 2 in the row
arr[row][0] === arr[row][2]) // compare square 1 and 3 in the row
{
// horizontal win--handle text for X/O win
if (arr[row][0] === 'x') {}
else if (arr[row][0] === 'o') {}
break;
}
}
for (let col = 0; col < 3; col++) {
if (arr[0][col] !== '' && // make sure it has an entered value
arr[0][col] === arr[1][col] && // compare square 1 and 2 in the column
arr[0][col] === arr[2][col]) // compare square 1 and 3 in the column
{
// vertical win--handle text for X/O win
if (arr[0][col] === 'x') {}
else if (arr[0][col] === 'o') {}
break;
}
}
if (arr[0][col] !== '' &&
arr[0][0] === arr[1][1] &&
arr[0][0] === arr[2][2] ||
arr[0][2] !== '' &&
arr[0][2] === arr[1][1] &&
arr[0][2] === arr[2][0])
{
// diagonal win--handle text for X/O win
if (arr[row][0] === 'x') {}
else if (arr[row][0] === 'o') {}
}Context
StackExchange Code Review Q#147167, answer score: 3
Revisions (0)
No revisions yet.