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();
\$\endgroup\$

2 Answers 2

Reset to default
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.

\$\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\$
    – Mario Sanchez Carrion
    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\$
    – I wrestled a bear once.
    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>

\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.