4
\$\begingroup\$

I have programmed a Tic Tac Toe game using Object Oriented JavaScript and ES6 Classes.

The program works as planned, but I wonder if some parts of the code can be made cleaner or more efficient.

I'm mostly concerned with the JS parts, not the HTML or CSS.

Here is the code:

index.html

<html>
 <head>
 <meta name="viewport" content="width=device-width, initial-scale=1">
 <link rel="stylesheet" href="main.css"> 
 </head>
 <body>
 <h2 id="title">Tic Tac Toe</h2>
 <div class="container" id="board">
 <div class="row">
 <div class="square" id="00"></div>
 <div class="square" id="01"></div>
 <div class="square" id="02"></div>
 </div>
 <div class="row">
 <div class="square" id="10"></div>
 <div class="square" id="11"></div>
 <div class="square" id="12"></div>
 </div>
 <div class="row">
 <div class="square" id="20"></div>
 <div class="square" id="21"></div>
 <div class="square" id="22"></div>
 </div>
 </div>
 <button id="new-game">New Game</button>
 <h3 id="message"></h3>
 <script src="objects.js"></script>
 <script src="main.js"></script>
 </body>
</html>

main.css

body {
 text-align: center;
 font-family: Arial, Helvetica, sans-serif;
}
.square {
 display: inline-block;
 width: 185px;
 height: 165px;
 margin: 3px;
 background-color: #bfbfbf;
 vertical-align: top;
 font-size: 700%;
 color: red;
}
.row {
 min-width: 600px ;
}
#new-game {
 margin: 10px;
}
#message {
 font-size: 3em;
}

objects.js

'use strict'
class Board {
 constructor(grid) {
 this.grid = [
 [new Square(), new Square(), new Square()],
 [new Square(), new Square(), new Square()],
 [new Square(), new Square(), new Square()]
 ];
 }
 isFull() {
 let count = 0;
 for (let i = 0; i < 3; i++) {
 for (let j = 0; j < 3; j++) {
 if (this.grid[i][j].state != "") { count++ }
 }
 }
 if (count == 9) {
 return true;
 }
 else {
 return false;
 }
 }
 check_win() {
 for (let i = 0; i < 3; i++) {
 if (this.grid[i][0].state == this.grid[i][1].state && this.grid[i][0].state == this.grid[i][2].state && this.grid[i][0].state != "") {
 print_winner();
 }
 }
 for (let j = 0; j < 3; j++) {
 if (this.grid[0][j].state == this.grid[1][j].state && this.grid[0][j].state == this.grid[2][j].state && this.grid[0][j].state != "") {
 print_winner();
 }
 }
 if ((this.grid[0][0].state == this.grid[1][1].state && this.grid[0][0].state == this.grid[2][2].state && this.grid[0][0].state != "") ||
 (this.grid[0][2].state == this.grid[1][1].state && this.grid[0][2].state == this.grid[2][0].state && this.grid[0][2].state != "")) {
 print_winner();
 }
 if (this.isFull()) {
 print_tie();
 }
 }
}
class Square {
 constructor(state) {
 this.state = "";
 }
 whoseTurn() {
 if (counter == 0) {
 return this.state = "x";
 }
 else {
 return this.state = "o";
 }
 }
}
class Player {
 constructor(symbol) {
 this.symbol = symbol;
 }
}
class Game {
 constructor() {
 this.board = new Board();
 this.players = [
 new Player("x"),
 new Player("o")
 ];
 }
}

main.js

'use strict'
var g = new Game();
var counter = 0;
function play() {
 let bd = document.getElementById('board');
 bd.addEventListener('click', (event) => {
 // console.log(event.target);
 event.target.innerHTML = g.players[counter].symbol;
 var squareNum = event.target.id.split('');
 var row = squareNum[0];
 var col = squareNum[1];
 g.board.grid[row][col].whoseTurn();
 g.board.check_win();
 if (counter == 0) { counter = 1 }
 else { counter = 0 }
 });
}
function print_winner() {
 let winner = g.players[counter].symbol;
 console.log(`${winner} wins`);
}
function print_tie() {
 console.log("It's a tie");
}
function play_again() {
 let bt = document.getElementById('new-game');
 bt.addEventListener('click', () => {
 for (let i = 0; i < 9; i++) {
 document.querySelectorAll('.square')[i].innerHTML = "";
 }
 g = new Game();
 });
}
play();
play_again();
Sᴀᴍ Onᴇᴌᴀ ♦
29.6k16 gold badges45 silver badges203 bronze badges
asked Dec 18, 2017 at 15:54
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$
  • When print_winner is called the game needs to end. As it is, if the board is full AND x wins it will print both "x wins" AND "it's a tie". The easiest way to do that is to return print_winner(); instead of just print_winner();.
  • Manually checking the adjacent squares by hardcoding the coords is a nice fast way to check for 3 in a row, but it's not easily expandable (say, if you wanted to make a 5-in-a-row tick tack toe), nor is it really easily readable. A better way to do it is to have a function that checks each of the adjacent squares and determines how many there are in a row.
  • The grid parameter to the Board constructor is never used.
  • You shouldn't be calling global functions (print_winner() and print_tie()) from your class. You should pass the functions into the class instead. See dependency injection.

Here's how I would have written the Board Class

class Board {
 constructor(onWinner, onTie) {
 this.print_winner = onWinner;
 this.print_tie = onTie;
 this.grid = [
 [new Square(), new Square(), new Square()],
 [new Square(), new Square(), new Square()],
 [new Square(), new Square(), new Square()]
 ];
 }
 isFull() {
 let count = 0;
 for (let i = 0; i < 3; i++) {
 for (let j = 0; j < 3; j++) {
 if (this.grid[i][j].state != "") { count++ }
 }
 }
 if (count == 9) {
 return true;
 }
 else {
 return false;
 }
 }
 getAdjacentCooords(x, y, direction){
 switch(direction){
 case "v": x++; break;
 case "h": y++; break;
 case "dd": x++; y++; break;
 case "da": x++; y--; break;
 }
 return {x:x, y:y};
 }
 getAdjacentSquareState(x, y, direction){
 var coords = this.getAdjacentCooords(x, y, direction);
 if(!this.grid[coords.x] || 
 !this.grid[coords.x][coords.y] || 
 this.grid[coords.x][coords.y].state === "") return false;
 return this.grid[coords.x][coords.y].state;
 }
 getSquenceCountStartingAt(x, y, direction){
 var sequence = 1;
 var state = this.grid[x][y].state;
 while(true){
 var nextState = this.getAdjacentSquareState(x, y, direction);
 if(state === nextState && nextState !== false) sequence++;
 else break;
 var coords = this.getAdjacentCooords(x, y, direction);
 x = coords.x; y = coords.y;
 }
 return sequence;
 }
 check_win() {
 var total_squares = 3;
 // vertical
 for (let i = 0; i < total_squares; i++) {
 if(this.getSquenceCountStartingAt(0, i, 'v') === total_squares) return this.print_winner();
 }
 // horizontal
 for (let i = 0; i < total_squares; i++) {
 if(this.getSquenceCountStartingAt(i, 0, 'h') === total_squares) return this.print_winner();
 }
 // diagonal ascending
 if(this.getSquenceCountStartingAt(0, 0, 'da') === total_squares) return this.print_winner();
 // diagonal descending
 if(this.getSquenceCountStartingAt(0, total_squares-1, 'dd') === total_squares) return this.print_winner();
 if (this.isFull()) {
 return this.print_tie();
 }
 }
}

Here's a running example of the re-written code.

answered Dec 18, 2017 at 20:47
\$\endgroup\$
2
  • \$\begingroup\$ Thanks! One question: what are the OnWinner and OnTie arguments in the Board constructor and where are they used? Do you have to pass them to create a new Board instance? \$\endgroup\$ Commented Dec 19, 2017 at 4:17
  • \$\begingroup\$ @MarioSanchezCarrion - They are set to this.print_winner and this.print_tie, respectively and used instead of the original global ones. See my last bullet point. \$\endgroup\$ Commented Dec 19, 2017 at 13:20
1
\$\begingroup\$

Return value from isFull()

if (count == 9) {
 return true;
}
else {
 return false;
}

This could be simplified by simply returning whether count is equal to 9. And to be sure that the count is an integer (which it should always be since it starts at 0 and only gets incremented by 1) use the strict equality operator (i.e. ===):

return count === 9;

Cache DOM elements

In function play_again() I see this code:

for (let i = 0; i < 9; i++) {
 document.querySelectorAll('.square')[i].innerHTML = "";
}

That means that there are 9 lookups on the Document Object Model (the DOM). Those can be take considerable time in the context of browser operations! A better approach would be store those elements in an array (well, actually it is a nodelist) as soon as the DOM is loaded:

//run this as soon as DOM is ready
const squares = document.querySelectorAll('.square');

Then when iterating over the elements, index into that array of elements:

for (let i = 0; i < 9; i++) {
 squares[i].innerHTML = "";
}

Event delegation

One could add a single click handler for the whole DOM using document.body (or wrap the game board and new game buttons in another element), then check if the click event target is the new game button and if so, reset the board, otherwise handle clicking on an element. A sample starter is below...

document.addEventListener('DOMContentLoaded', function() {
 //run this as soon as DOM is ready
 const squares = document.querySelectorAll('.square');
 
 //set up game
 
 //add click handler
 document.body.addEventListener('click', function() {
 if (event.target.id == 'new-game') {
 for (let i = 0; i < squares.length; i++) {
 squares[i].innerHTML = "";
 }
 }
 //otherwise call function to make a player move
 //...
 });
});
body {
 text-align: center;
 font-family: Arial, Helvetica, sans-serif;
}
.square {
 display: inline-block;
 width: 185px;
 height: 165px;
 margin: 3px;
 background-color: #bfbfbf;
 vertical-align: top;
 font-size: 700%;
 color: red;
}
.row {
 min-width: 600px;
}
#new-game {
 margin: 10px;
}
#message {
 font-size: 3em;
}
<h2 id="title">Tic Tac Toe</h2>
<div class="container" id="board">
 <div class="row">
 <div class="square" id="00">X</div>
 <div class="square" id="01">O</div>
 <div class="square" id="02">O</div>
 </div>
 <div class="row">
 <div class="square" id="10">O</div>
 <div class="square" id="11"></div>
 <div class="square" id="12"></div>
 </div>
 <div class="row">
 <div class="square" id="20">X</div>
 <div class="square" id="21"></div>
 <div class="square" id="22"></div>
 </div>
</div>
<button id="new-game">New Game</button>
<h3 id="message"></h3>

answered Dec 18, 2017 at 16:56
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.