5
\$\begingroup\$

I am a beginner programmer. In a prototype function of the game function, called game.prototype.restart, I repeated some lines of code to reset variables I used to keep track of scores that were in its parent function. I'm not really sure right now how I could've made a restart button for my game another way, but it works.

    'use strict'

function game () {
  this.player = null;
  this.computer = null;
  this.turns = 0;
  this.playerChoices = [];
  this.computerChoices = [];
  this.squares = document.querySelectorAll('.play');
  this.spotsLeft = [0,1,2,3,4,5,6,7,8];
  this.winningCombos = [
    [0, 1, 2],[6, 7, 8],[0, 3, 6],[1, 4, 7],[2, 5, 8],
    [0, 4, 8],[2, 4, 6], [3,4,5]
  ];

  for(let i = 0; i < this.squares.length; i++){
    this.squares[i].addEventListener('click', (function() {
      this.playerMove(i);
    }).bind(this));
  }
}

game.prototype.chooseSymbol = function (symbol) {
    this.player = symbol;
    //choose computers symbol
    this.computer = symbol === 'x' ? 'o' : 'x';

    document.querySelector('.sign').style.visibility = 'hidden';

}

game.prototype.playerMove = function (i) {
    if(this.squares[i].classList.contains('play')){

        this.squares[i].textContent = this.player;

        this.turns++; console.log(this.turns);

        //remove to prevent overwriting text content on computer move and make player move array
        this.squares[i].classList.remove('play');

        //remove spot player chose to prevent computer from moving here
        let index = this.spotsLeft.indexOf(i);

        this.spotsLeft.splice(index, 1);

        //push to array to compare for win
        this.playerChoices.push(i); 

        this.checkWin(this.playerChoices, 'Player');

       if(this.checkWin(this.playerChoices, 'Player') === false){
           this.computerMove();
       }

    }
}

//chooses spot randomly
game.prototype.computerMove = function () {
    let random = this.spotsLeft[Math.floor(Math.random() * this.spotsLeft.length)]

    this.squares[random].textContent = this.computer;

    //remove to prevent choosing again
    let index = this.spotsLeft.indexOf(random);

    this.spotsLeft.splice(index, 1);

    this.squares[random].classList.remove('play');

    this.computerChoices.push(random);

    this.checkWin(this.computerChoices, 'Computer');

}

//compare player or computer arrays to check for win or tie
game.prototype.checkWin = function (player, name) {
    console.log(name);

    let win = this.winningCombos.some((ar) => ar.every((c) => player.includes(c)));

    if( win === true){
        document.querySelector('.result').style.visibility = 'visible';
        document.querySelector('#tie').textContent =  `${name} wins!`
    } else if (this.turns === 5){
        document.querySelector('.result').style.visibility = 'visible';
        document.querySelector('#tie').textContent =   `Tie!`
    }

    return Boolean(win);

}

game.prototype.restart = function () {

    document.querySelector('.result').style.visibility = 'hidden';

    document.querySelector('.sign').style.visibility = 'visible';

    this.player = null;

    this.computer = null;

    this.turns = 0;

    this.playerChoices = [];

    this.computerChoices = [];

    this.spotsLeft = [0,1,2,3,4,5,6,7,8];

    for(let i = 0; i < this.squares.length; i++){
        this.squares[i].classList.add('play');
        this.squares[i].textContent = ' ';
    }
}


const ticTacToe = new game();

document.querySelector('#playx').addEventListener('click', () => {
    ticTacToe.chooseSymbol('x')
});

document.querySelector('#playo').addEventListener('click', () => {
    ticTacToe.chooseSymbol('o')
});

document.querySelector('#replay').addEventListener('click', () => {
    ticTacToe.restart();
})


'use strict'

function game() {
  this.player = null;
  this.computer = null;
  this.turns = 0;
  this.playerChoices = [];
  this.computerChoices = [];
  this.squares = document.querySelectorAll('.play');
  this.spotsLeft = [0, 1, 2, 3, 4, 5, 6, 7, 8];
  this.winningCombos = [
    [0, 1, 2],
    [6, 7, 8],
    [0, 3, 6],
    [1, 4, 7],
    [2, 5, 8],
    [0, 4, 8],
    [2, 4, 6],
    [3, 4, 5]
  ];

  for (let i = 0; i < this.squares.length; i++) {
    this.squares[i].addEventListener('click', (function() {
      this.playerMove(i);
    }).bind(this));
  }
}

game.prototype.chooseSymbol = function(symbol) {
  this.player = symbol;
  //choose computers symbol
  this.computer = symbol === 'x' ? 'o' : 'x';

  document.querySelector('.sign').style.visibility = 'hidden';

}

game.prototype.playerMove = function(i) {
  if (this.squares[i].classList.contains('play')) {

    this.squares[i].textContent = this.player;

    this.turns++;
    console.log(this.turns);

    //remove to prevent overwriting text content on computer move and make player move array
    this.squares[i].classList.remove('play');

    //remove spot player chose to prevent computer from moving here
    let index = this.spotsLeft.indexOf(i);

    this.spotsLeft.splice(index, 1);

    //push to array to compare for win
    this.playerChoices.push(i);

    this.checkWin(this.playerChoices, 'Player');

    if (this.checkWin(this.playerChoices, 'Player') === false) {
      this.computerMove();
    }

  }
}

//chooses spot randomly
game.prototype.computerMove = function() {
  let random = this.spotsLeft[Math.floor(Math.random() * this.spotsLeft.length)]

  this.squares[random].textContent = this.computer;

  //remove to prevent choosing again
  let index = this.spotsLeft.indexOf(random);

  this.spotsLeft.splice(index, 1);

  this.squares[random].classList.remove('play');

  this.computerChoices.push(random);

  this.checkWin(this.computerChoices, 'Computer');

}

//compare player or computer arrays to check for win or tie
game.prototype.checkWin = function(player, name) {
  console.log(name);

  let win = this.winningCombos.some((ar) => ar.every((c) => player.includes(c)));

  if (win === true) {
    document.querySelector('.result').style.visibility = 'visible';
    document.querySelector('#tie').textContent = `${name} wins!`
  } else if (this.turns === 5) {
    document.querySelector('.result').style.visibility = 'visible';
    document.querySelector('#tie').textContent = `Tie!`
  }

  return Boolean(win);

}

game.prototype.restart = function() {

  document.querySelector('.result').style.visibility = 'hidden';

  document.querySelector('.sign').style.visibility = 'visible';

  this.player = null;

  this.computer = null;

  this.turns = 0;

  this.playerChoices = [];

  this.computerChoices = [];

  this.spotsLeft = [0, 1, 2, 3, 4, 5, 6, 7, 8];

  for (let i = 0; i < this.squares.length; i++) {
    this.squares[i].classList.add('play');
    this.squares[i].textContent = ' ';
  }
}


const ticTacToe = new game();

document.querySelector('#playx').addEventListener('click', () => {
  ticTacToe.chooseSymbol('x')
});

document.querySelector('#playo').addEventListener('click', () => {
  ticTacToe.chooseSymbol('o')
});

document.querySelector('#replay').addEventListener('click', () => {
  ticTacToe.restart();
})
* {
  margin: 0;
  padding: 0;
  box-sizing: border-box;
}

body {
  background-color: rgba(244, 100, 0, 0.1);
}

.centered {
  position: fixed;
  top: 50%;
  left: 50%;
  transform: translate(-50%, -50%);
}


/* choose x or o block */

.shade {
  z-index: 1;
  position: absolute;
  height: 100%;
  width: 100%;
  display: flex;
  justify-content: center;
  align-items: center;
}

.box {
  padding: 1.250rem 1.875rem;
  height: auto;
  background-color: azure;
  top: 6em;
}


/* tic tac toe Board */

table {
  z-index: 0;
  border-collapse: collapse;
  table-layout: fixed;
  display: flex;
  justify-content: center;
}

td {
  width: 188.8px;
  height: 188.8px;
  border: 6px solid #222;
  text-align: center;
  vertical-align: middle;
  font-size: 100px;
}

td {
  border: 6px solid #222;
}

td:first-of-type {
  border-left-color: transparent;
  border-top-color: transparent;
}

td:nth-of-type(2) {
  border-top-color: transparent;
}

td:nth-of-type(3) {
  border-right-color: transparent;
  border-top-color: transparent;
}

tr:nth-of-type(3) td {
  border-bottom-color: transparent;
}


/* replay button */

.result {
  visibility: hidden;
}

.replayBox {
  display: flex;
  align-items: center;
  justify-content: center;
  flex-direction: column;
  flex-wrap: nowrap
}
<!-- choose sign -->
<div class='shade sign'>
  <div class='box'>
    <button id='playx'>X</button>
    <button id='playo'>o</button>
  </div>
</div>
<!-- win or tie -->
<div class='shade result'>
  <div class='box replayBox'>
    <h3 id='tie'></h3>
    <button id='replay'>Replay?</button>
  </div>
</div>
<!-- board -->
<table id='board' class='centered'>
  <tr>
    <td id='0' class='play'></td>
    <td id='1' class='play'></td>
    <td id='2' class='play'></td>
  </tr>
  <tr>
    <td id='3' class='play'></td>
    <td id='4' class='play'></td>
    <td id='5' class='play'></td>
  </tr>
  <tr>
    <td id='6' class='play'></td>
    <td id='7' class='play'></td>
    <td id='8' class='play'></td>
  </tr>
</table>

\$\endgroup\$

2 Answers 2

Reset to default
3
\$\begingroup\$

You can put the duplicate code in it's own function (or just keep it in restart) and then call that function from game() as well.

function game () {
    // set all positions to 0
    this.restart()
}
\$\endgroup\$
0
3
\$\begingroup\$

Main question

I'm not really sure right now how I could've made a restart button for my game another way, but it works

Like Kokodoko's answer suggests, the common code can be abstracted. I would abstract the common code to a separate method like setupInitialState:

game.prototype.setupInitialState = function() {
  this.player = null;
  this.computer = null;
  this.turns = 0;
  this.playerChoices = [];
  this.computerChoices = [];
  this.spotsLeft = [0,1,2,3,4,5,6,7,8];
}

Then call that method when starting and restarting the game:

function game () {
  this.squares = document.querySelectorAll('.play');
  this.setupInitialState();

  for(let i = 0; i < this.squares.length; i++){
    this.squares[i].addEventListener('click', (function() {
      this.playerMove(i);
    }).bind(this));
  }
}

And similarly in the restart method:

game.prototype.restart = function () {

    document.querySelector('.result').style.visibility = 'hidden';

    document.querySelector('.sign').style.visibility = 'visible';
    this.setupInitialState();

    for(let i = 0; i < this.squares.length; i++){
        this.squares[i].classList.add('play');
        this.squares[i].textContent = ' ';
    }
}

Other Review points

JS

A lot of the suggestions below come from experience writing Javascript over the past decade, getting feedback from colleagues, and reading various posts online like this one.

Bind methods and create partial functions to simplify callback functions

Instead of adding a click handler with an anonymous function bound to this that just calls a method:

this.squares[i].addEventListener('click', (function() {
      this.playerMove(i);
    }).bind(this));

make a partial function (still using Function.bind())

this.squares[i].addEventListener('click', this.playerMove.bind(this, i));

And similarly for the other click handlers: 

document.querySelector('#playx').addEventListener('click', () => {
    ticTacToe.chooseSymbol('x')
});

Can be simplified with the same technique:

document.querySelector('#playx').addEventListener('click', ticTacToe.chooseSymbol.bind(ticTacToe,'x'));

Use getElementsByClassName() instead of querySelectorAll(), getElementById() instead of querySelector()

In most browsers it would generally be quicker to fetch the game squares using document.getElementsByClassName(), but that would return a live collection (refer to this SO answer for an explanation). It doesn't appear that the class play is used for styling so it might be simpler to use event delegation to add a click handler to the table (which can be selected using document.getElementById('board') - should be quicker than document.querySelector() for a non-complex query).

Remove excess call to checkWin()

The two calls at the end of playerMove() to checkWin() below could be combined to one:

this.checkWin(this.playerChoices, 'Player');

if (this.checkWin(this.playerChoices, 'Player') !== false) {
  this.computerMove();
}

(or optionally the value from the first call could be stored in a variable (or constant using const) to be used in the if condition).

HTML

Use a more appropriate id value for tie

That element is used to display the outcome message of the game - a more appropriate value for the id attribute might be message or outcome.

CSS

Remove excess ruleset

There appears to be an excess ruleset:

td {
  border: 6px solid #222;
}

That can be removed, since the ruleset above that contains the same border style.

hidden class

Instead of adding the style visibility: hidden to the elements with class result, make it a class like hidden that can be added and removed in order to hide various elements (e.g. the outcome message).

Rewrite

Below is how I would simplify your code. Hopefully the explanations above support all the changes, but there may be a few things that you might have to research in order to figure them out.

I also removed the class play from the table cells and updated the code to check if that class exists on the table cells - instead, the spotsLeft member variable can be used for that discernment. Then there is no need to remove the class and later add it when restarting.

'use strict'

function game() {
  this.board = document.getElementById('board');
  this.squares = this.board.getElementsByTagName('td');
  this.chooser = document.getElementById('chooser');
  this.message = document.getElementById('message');
  this.result = document.getElementById('result');
  this.board.addEventListener('click', clickEvent => this.playerMove(parseInt(clickEvent.target.id, 10)));

  this.setupInitialState();

}
game.prototype.setupInitialState = function() {
  this.player = null;
  this.computer = null;
  this.turns = 0;
  this.playerChoices = [];
  this.computerChoices = [];
  this.spotsLeft = [0, 1, 2, 3, 4, 5, 6, 7, 8];
}
//could be a constant - using const
game.prototype.winningCombos = [
  [0, 1, 2],
  [6, 7, 8],
  [0, 3, 6],
  [1, 4, 7],
  [2, 5, 8],
  [0, 4, 8],
  [2, 4, 6],
  [3, 4, 5]
];
game.prototype.chooseSymbol = function(symbol) {
  this.player = symbol;
  //choose computers symbol
  this.computer = symbol === 'x' ? 'o' : 'x';
  this.chooser.classList.add('hidden');
}

game.prototype.playerMove = function(i) {
  if (this.spotsLeft.includes(i)) {
    this.squares[i].textContent = this.player;

    this.turns++;

    //remove spot player chose to prevent computer from moving here
    let index = this.spotsLeft.indexOf(i);

    this.spotsLeft.splice(index, 1);

    //push to array to compare for win
    this.playerChoices.push(i);

    if (!this.checkWin(this.playerChoices, 'Player')) {
      this.computerMove();
    }

  }
}

//chooses spot randomly
game.prototype.computerMove = function() {
  let random = this.spotsLeft[Math.floor(Math.random() * this.spotsLeft.length)]

  this.squares[random].textContent = this.computer;

  //remove to prevent choosing again
  let index = this.spotsLeft.indexOf(random);

  this.spotsLeft.splice(index, 1);

  this.squares[random].classList.remove('play');

  this.computerChoices.push(random);

  this.checkWin(this.computerChoices, 'Computer');

}

//compare player or computer arrays to check for win or tie
game.prototype.checkWin = function(player, name) {
  let win = this.winningCombos.some((ar) => ar.every((c) => player.includes(c)));

  if (win) {
    this.result.classList.remove('hidden');
    this.message.textContent = `${name} wins!`;
  } else if (this.turns === 5) {
    this.result.classList.remove('hidden');
    this.message.textContent = 'Tie';
  }

  return win;
}

game.prototype.restart = function() {
  this.result.classList.add('hidden');
  this.chooser.classList.remove('hidden');
  this.setupInitialState();

  for (let i = 0; i < this.squares.length; i++) {
    this.squares[i].textContent = ' ';
  }
}


const ticTacToe = new game();

document.querySelector('#playx').addEventListener('click', ticTacToe.chooseSymbol.bind(ticTacToe, 'x'));

document.querySelector('#playo').addEventListener('click', ticTacToe.chooseSymbol.bind(ticTacToe, 'o'));

document.querySelector('#replay').addEventListener('click', ticTacToe.restart.bind(ticTacToe));
* {
  margin: 0;
  padding: 0;
  box-sizing: border-box;
}

body {
  background-color: rgba(244, 100, 0, 0.1);
}

.centered {
  position: fixed;
  top: 50%;
  left: 50%;
  transform: translate(-50%, -50%);
}


/* choose x or o block */

.shade {
  z-index: 1;
  position: absolute;
  height: 100%;
  width: 100%;
  display: flex;
  justify-content: center;
  align-items: center;
}

.box {
  padding: 1.250rem 1.875rem;
  height: auto;
  background-color: azure;
  top: 6em;
}


/* tic tac toe Board */

table {
  z-index: 0;
  border-collapse: collapse;
  table-layout: fixed;
  display: flex;
  justify-content: center;
}

td {
  width: 188.8px;
  height: 188.8px;
  border: 6px solid #222;
  text-align: center;
  vertical-align: middle;
  font-size: 100px;
}

td {
  border: 6px solid #222;
}

td:first-of-type {
  border-left-color: transparent;
  border-top-color: transparent;
}

td:nth-of-type(2) {
  border-top-color: transparent;
}

td:nth-of-type(3) {
  border-right-color: transparent;
  border-top-color: transparent;
}

tr:nth-of-type(3) td {
  border-bottom-color: transparent;
}


/* replay button */

.replayBox {
  display: flex;
  align-items: center;
  justify-content: center;
  flex-direction: column;
  flex-wrap: nowrap
}

.hidden {
  visibility: hidden;
}
<div class='shade sign' id="chooser">
  <div class='box'>
    <button id='playx'>X</button>
    <button id='playo'>o</button>
  </div>
</div>
<!-- win or tie -->
<div class='shade hidden' id="result">
  <div class='box replayBox'>
    <h3 id='message'></h3>
    <button id='replay'>Replay?</button>
  </div>
</div>
<!-- board -->
<table id='board' class='centered'>
  <tr>
    <td id='0'></td>
    <td id='1'></td>
    <td id='2'></td>
  </tr>
  <tr>
    <td id='3'></td>
    <td id='4'></td>
    <td id='5'></td>
  </tr>
  <tr>
    <td id='6'></td>
    <td id='7'></td>
    <td id='8'></td>
  </tr>
</table>

\$\endgroup\$
1
  • \$\begingroup\$ thank you very much for the feedback I really appreciate it! \$\endgroup\$
    – icewizard
    Commented Mar 31, 2018 at 2:33

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