Feedback on Tic Tac Toe game

Feedback on Tic Tac Toe game
0.0 0

#1

Hi, I just finished up my Tic Tac Toe game. It took me 132 hours. I learned a lot, but coding the different levels was grueling and tedious to say the least. I didn’t want to use the minimax algorithm as I felt the solution wouldn’t be my own, but after so much time spent I wonder if this is a better approach in general. I’m sure I’d learn a lot either way, but I’ve been continually astounded about how much time these projects take to build. The advanced projects are supposed to take ~140hrs total, and I spent that on TTT alone. I’m almost a year in to FCC, working on it nearly full time, and I’m only 1/3 of the way done with the curriculum. Anyway, any feedback is welcome, especially if you have any ideas on how to approach the project(s) more efficiently.
Thanks!


#2

Very well done :+1: Think you should also add some option to restart the game so you can choose different levels of difficulty without the need to reload the page.


#3

I tested a few rounds and the computer seems to play the correct strategy on Pro setting, so congrats on a well design game. I like the interaction with the game.

Now, you just need to go back and make your code DRY. You have many code sections and/or functions with 99% duplicate code. The first example of non-DRY code is found in lines 3-30 with the following:

var ttt = {
  level: '',
  board: [null, null, null, null, null, null, null, null, null],
  boardString: function stringifyBoard() {
    return JSON.stringify(this.board);
  },
  openPositions: [],
  players: '',
  player1_symbol: '',
  player2_symbol: '',
  computer_symbol: '',
  player1_turn: false,
  moveCount: 0,
  lastMove: '',
  draw: false,
  winner: '',
  winningPositions: [],
  player1_score: 0,
  player2_score: 0,
  computer_score: 0,
  gamesPlayed: 0,
  gamesTied: 0,
};

// Winning line combinations
var winLines = [ [0, 1, 2], [3, 4, 5], [6, 7, 8], // horiz. ['X', 'X', 'X']
                 [0, 3, 6], [1, 4, 7], [2, 5, 8], // vert.
                 [0, 4, 8], [2, 4, 6] ];          // diag.

This same code appears again in your reset function. Just declare ttt and winLines as global variables and then call the reset function at the beginning to be able to delete about lines of code.

There are so many other places where just creating a stand-alone function would allow you to delete 300-400 lines of code.


#4

In css as you have used a lot of ids that are unique instead of div#app{css code in here} you may just use #app{....}


#5

@sorinr There is a RESET tab/button at the top right-hand corner of the board.


#6

@randelldawson @Feldbot here how i see it on my monitor on editor and full page view:


#7

Yeah, the body needs about 30px of top margin, so you can see the RESET tab/button. Change the Zoom to 80% and you should be able to see it.


#8

Thanks for the feedback. I do need to try to redesign so it fits the screen better. The same problem happens on mobile.


#9

Thanks, that is good feedback. I did wonder how to better design the restart function and what you suggest makes sense. Are any other areas screaming out DRY to you? I’d be curious how to cut out 300-400 lines of code!


#10

Just look through your code. Where ever you see a section or function which is very similar except maybe a couple of differences, there is an opportunity to make it DRY with a stand-alone function or the addition of some extra if statements or some extra generic variables.

For example, look at the following section of code:

      // Setup header
      if (ttt.players === 1) {
        $("div#stats").html(`
          <div id="players">
            <h3 id="player1">${ttt.player1_symbol} - Player 1 </h3>
            <h3 id="computer">${ttt.computer_symbol} - Computer </h3>
          </div>
          <div id="score">
            <h4>Wins:</h4>
            <h3><span id="player1_score" class="score">0</span></h3>
            <h3><span id="computer_score" class="score">0</span></h3>
          </div>
          <div id="smallStats">
            <h4 id="gamesTied"> Draws: 0<h4>
            <h4 id="gamesPlayed">Games: 0</h4>
          </div>`);
      } else {
        $("div#stats").html(`
          <div id="players">
            <h3 id="player1">${ttt.player1_symbol} - Player 1 </h3>
            <h3 id="player2">${ttt.player2_symbol} - Player 2 </h3>
          </div>
          <div id="score">
            <h4>Wins:</h4>
            <h3><span id="player1_score" class="score">0</span></h3>
            <h3><span id="player2_score" class="score">0</span></h3>
          </div>
          <div id="smallStats">
            <h4 id="gamesTied">Draws: 0<h4>
            <h4 id="gamesPlayed">Games: 0</h4>
          </div>`);
      }

The only differences between the if block of code and the else block of code are the following lines of code.
if

            <h3 id="computer">${ttt.computer_symbol} - Computer </h3>

else

            <h3 id="player2">${ttt.player2_symbol} - Player 2 </h3>

See if you can figure out how you have the same end result, but only have the one section of code. There are many different ways you could approach it. If you get stuck, show us what you have tried and we will point you in the right direction.

Another good example of how to make your code DRY is the following section:

        // Render symbol on board
        if (ttt.player1_turn) {
          $(this)[0].innerHTML = ttt.player1_symbol;
        } else {
          $(this)[0].innerHTML = ttt.player2_symbol;
        }

        // Toggle player turn
        if (ttt.player1_turn) {
          ttt.player1_turn = false;
        } else {
          ttt.player1_turn = true;
        }

There is no reason to have to of the same if statements. For the if block of code, you can simply update the innerHTML with the player1_symbol and then in the next line assign false to player1_turn. For the else block of code, you can simply update the innerHTML with the player2_symbol and then in the next line assign true to player1_turn.

Or another option is to use the ternary operator for the first if/else (see below)

$(this)[0].innerHTML = ttt.player1_turn ? ttt.player1_symbol : ttt.player2_symbol;

and then just flip the value of ttt.player1_turn by negating the current value of the variable (see below).

ttt.player1_turn = !ttt.player1_turn; // eliminates the need for an if/else statement

#11

Hi Randell,
I’m now working on the Simon app and running into the same issue trying to figure out how to reset the app in a DRY manner. I still don’t understand how to not duplicate when resetting. For example, when I initialize the app everything is set to their defaults in global variables ttt and winLines. As the game progresses, theses variable key values are overwritten, so calling reset on them then would result in resetting with the current state of ttt and winLines if it was just one object. So it seems like I do need to duplicate those objects to always keep a copy of the initial state available. I fell like I am still missing something in understanding how to write the init state only once and be able to reset and overwrite that same object. Can you help clarify what you meant by calling reset() at the beginning and how that would help eliminate code?