3
\$\begingroup\$

I have this tic tac toe game which is very old project of mine, though I made some changes on this to improve the script to perform better.

This game is functional, Any logic improvements or other suggestions are greatly appreciated.

// tic tac toe v0.2, revised on 20th October, 2018;

var turn = 0; //is used for checking players turns...
var box = document.getElementsByClassName("box"); // one variable for all nine boxes...
var board = document.getElementById("board");

var modalParent = document.getElementById('modal-container');
var modal = modalParent.getElementsByClassName('custom-modal')[0];

//this function rotates the board randomly ...
function rotateBoard() {
  var rotator = ["transform:rotate(0deg)", "transform:rotate(90deg)", "transform:rotate(180deg)", "transform:rotate(270deg)"];
  board.setAttribute("style", rotator[Math.floor(Math.random() * 4)]);
}

//  this function will check which palyer wins....
//	when we set the value of each X to 2, all winning chances are here like this.
//  result of each row/column/slash is 6 when X wins.
//	
//      6    6    6    6
//      "    "    "  //        
//	    2 |  2  | 2   =  6
//	 -----+-----+----
//	    2 |  2  | 2   =  6
//	 -----+-----+----
//      2 |  2  | 2   =  6
//      
// setting all O to value 5 will make a winning number to 15 from all sides, unless these seven results 
// is equal to 6 or 15 , its a tie match. lets see if the function works or not ....
//
//	   15    15   15   15 
//	    "    "    "  //
//	    5 |  5  | 5    = 15
//	 -----+-----+----
//	    5 |  5  | 5    = 15
//	 -----+-----+----
//      5 |  5  | 5    = 15


// this function handles the win results inside a popup;
var popUpWindow = function(playerImagePosition) {
  modalParent.style.opacity = '1';
  modalParent.style.zIndex = '100';
  modal.style.zIndex = '200';
  if (playerImagePosition === 'tie') {
    modal.getElementsByTagName('h2')[0].innerHTML = "It's a Tie";
  } else {
    modal.getElementsByClassName('player-won')[0].style.backgroundPositionX = playerImagePosition;
  }
  modal.getElementsByTagName('button')[0].addEventListener('click', function() {
    window.location.reload(true);
  });
};

function winCheck() {
  var rowOne = parseInt(box[0].dataset.playerType) +
    parseInt(box[1].dataset.playerType) +
    parseInt(box[2].dataset.playerType);
  var rowTwo = parseInt(box[3].dataset.playerType) +
    parseInt(box[4].dataset.playerType) +
    parseInt(box[5].dataset.playerType);
  var rowThree = parseInt(box[6].dataset.playerType) +
    parseInt(box[7].dataset.playerType) +
    parseInt(box[8].dataset.playerType);
  var colOne = parseInt(box[0].dataset.playerType) +
    parseInt(box[3].dataset.playerType) +
    parseInt(box[6].dataset.playerType);
  var colTwo = parseInt(box[1].dataset.playerType) +
    parseInt(box[4].dataset.playerType) +
    parseInt(box[7].dataset.playerType);
  var colThree = parseInt(box[2].dataset.playerType) +
    parseInt(box[5].dataset.playerType) +
    parseInt(box[8].dataset.playerType);
  var backSlash = parseInt(box[0].dataset.playerType) +
    parseInt(box[4].dataset.playerType) +
    parseInt(box[8].dataset.playerType);
  var forwardSlash = parseInt(box[2].dataset.playerType) +
    parseInt(box[4].dataset.playerType) +
    parseInt(box[6].dataset.playerType);

  var possibilities = [rowOne, rowTwo, rowThree, colOne, colTwo, colThree, backSlash, forwardSlash];

  // like explained above comments with diagram, any item from the above array should return 1 or 2 if a player
  // wins, it can return 2 because a player can sometimes win from 2 possible lines maximum;
  var xWin = possibilities.filter(scope => scope === 6);
  var oWin = possibilities.filter(scope => scope === 15);
  var tie = possibilities.filter(scope => isNaN(scope));

  // now take care of who won the game
  if (xWin.length === 1 || xWin.length === 2) {
    popUpWindow('200%');
  } else if (oWin.length === 1 || oWin.length === 2) {
    popUpWindow('100%');
  } else if (tie.length === 0 && xWin.length === 0 && oWin.length === 0) {
    popUpWindow('tie');
  }

}

var turnCheck = function(event) {
  if (event.target.classList.contains('box')) {
    if (event.target.getAttribute('data-player-type') === null) {
      event.target.setAttribute('data-player-type', (turn % 2 === 0) ? 2 : 5);
      event.target.style.backgroundPosition = (turn % 2 === 0) ? '200% 0' : '100% 0';
      turn++;
      winCheck();
    }
  }
};

board.addEventListener('click', turnCheck);

// only for personal portfolio page;
document.body.addEventListener("dblclick", function reload() {
  location.reload(true);
});

// rotate the board when window loads;
rotateBoard();
* {
  margin: 0;
  padding: 0;
  box-sizing: border-box;
  cursor: default
}

body {
  background-color: transparent;
  font-family: monospace;
  max-width: 1280px;
  margin: 80px auto
}

body h1 {
  margin-bottom: 10px
}

body .boundary {
  width: 500px;
  height: 500px
}

body .boundary .board {
  width: 100vw;
  height: 100vw;
  background-image: url('https://towkir.github.io/tictactoe/dist/images/boardback.svg');
  background-size: 100%;
  max-width: 500px;
  max-height: 500px
}

body .boundary .board .box {
  height: 33.33%;
  width: 33.33%;
  float: left;
  background-image: url('https://towkir.github.io/tictactoe/dist/images/players.png');
  background-size: 300%
}

body #controls,
body #tictactoe {
  display: block;
  width: 100%;
  max-width: 600px;
  height: 220px;
  margin: 30px 0
}

body #controls {
  height: 120px
}

body #tictactoe table td a {
  padding: 1px 5px;
  background: rgba(128, 128, 128, .3);
  border-radius: 2px;
  text-decoration: none;
  color: inherit;
  cursor: pointer
}

body #tictactoe table td a:active,
body #tictactoe table td a:focus,
body #tictactoe table td a:hover {
  background: rgba(128, 128, 128, .5);
  outline: 0;
  color: inherit
}

body #modal-container {
  background-color: rgba(0, 0, 0, .8);
  position: fixed;
  top: 0;
  left: 0;
  width: 100%;
  height: 100vh;
  display: block;
  z-index: -100;
  opacity: 0;
  transition: opacity .3s ease-in-out
}

body #modal-container .custom-modal {
  background-color: #fff;
  position: fixed;
  min-width: 300px;
  left: 50%;
  top: 40%;
  border-radius: 4px;
  z-index: -200;
  padding: 20px;
  transform: translate(-50%, -50%)
}

body #modal-container .custom-modal h2 {
  font-size: 60px
}

body #modal-container .custom-modal h2 .player-won {
  width: 80px;
  height: 80px;
  display: inline-block;
  vertical-align: middle;
  background-image: url('https://towkir.github.io/tictactoe/dist/images/players.png');
  background-size: 240px;
  background-position-x: 0
}

body #modal-container .custom-modal button {
  display: block;
  border: 0;
  background-color: #90ee90;
  width: 100%;
  padding: 10px;
  font-family: monospace;
  font-size: 20px;
  margin-top: 20px;
  cursor: pointer
}
<div class="boundary" id="boundary">
  <div class="board" id="board" style="transform:rotate(180deg)">
    <div class="box"></div>
    <div class="box"></div>
    <div class="box"></div>
    <div class="box"></div>
    <div class="box"></div>
    <div class="box"></div>
    <div class="box"></div>
    <div class="box"></div>
    <div class="box"></div>
  </div>
</div>
<div id="tictactoe">
  <h1>Tic Tac Toe</h1>
  <p>Among all the games, Tic Tac Toe seems to be the simplest, and this one is the simplest of all Tic Tac Toes out there. This is in two player mode, so the computer won't bother you at all. <br> Have fun with Noughts and Crosses. </p>
  <br>
  <table>
    <tbody>
      <tr>
        <td>Technology Used:</td>
        <td>HTML, CSS, JavaScript.</td>
      </tr>
      <tr>
        <td>Difficulty:</td>
        <td>What ? why would it be difficult ??</td>
      </tr>
      <tr>
        <td>Source Code:</td>
        <td>See the source code on <a href="https://github.com/towkir/tictactoe" target="_blank">This GitHub
                Repo</a></td>
      </tr>
    </tbody>
  </table>
</div>
<div id="controls">
  <h1>Game Controls</h1>
  <p><b>Reset:</b> Double click on the gamefield to reset to new game.</p>
</div>

<div id="modal-container">
  <div class="custom-modal">
    <h2><span class="player-won"></span> wins</h2>
    <button type="button" id="reload">Play Again</button>
  </div>
</div>

\$\endgroup\$
5
\$\begingroup\$

Style and code

  • Numeric styles can be set using Number. eg modalParent.style.opacity = '1'; can be modalParent.style.opacity = 1;

  • To convert a string to a number use Number rather than parseInt, or coerce the value eg const foo = "1" * 1

  • Good code style's most important attribute is consistency. If you use a particular style use the same style throughout the code.

    When defining a function you switch between the form function declaration (function name(){) and function expression (const name = function() {). Be consistent and use function declarations.

  • When setting an element/node content and it is just text use the textContent property, it does not require a re-flow and is thus more efficient then using innerHTML which forces parsing and re-flow of the page.

  • It is good practice to ensure that element.id(*1) are unique to the page. If you do this then you can then use direct element reference(*4) to access elements without the need to cache or query the DOM.

    eg <div id="myDiv"></div> <script> myDiv.textContent = "hello world" </script>

  • Try to keep the code DRY (don't repeat yourself). The function winCheck is very WET (write everything twice) and can be simplified (see example).

  • Elements have a dataset property. It is intended as a custom property that is part of the markup, changing a dataset property changes the markup.

    dataset is not intended as a means of inter-script communication. Elements as with all JS object are highly polymorphic(*2)(*3) and as such you can add and remove properties as required. See example.

    If you do access dataset properties in JS you should use the direct property name reference rather than indirectly via the setAttribute function which is only intended to be to access undefined (Not in the DOM API) DOM properties. eg <div id="myDiv" data-foo-bar="0"></div><script> const val = myDiv.dataset.fooBar; myDiv.dataset.fooBar = 2 </script>

  • You have a variety of magic numbers throughout the code. Try to collect all constants in one place and name them appropriately.

  • Always use the simplest code form. eg There are 6 redundant characters in the expression (turn % 2 === 0) ? 2 : 5 the brackets are redundant turn % 2 === 0 ? 2 : 5 Invert the condition and test for truthy turn % 2 ? 5 : 2

  • window is the default object (the globalThis) you only need to use it under very specific needs. eg window.location.reload(true); is the same as location.reload(true);

  • Use plurals for naming arrays or array like objects. eg box should be boxes

Logic

I do not get why you rotate the board, visually, gameplay wise, and code logic, it makes no difference, so why do it?

Example

The example is JS and (HTML to show reference associations) only as it is too much work to clean up the CSS for what is already a long answer.

I have made up CSS classes where needed rather than set style properties inline.

reloadButton.addEventListener("click", () => location.reload(true));
board.addEventListener("click", turnCheck);
document.body.addEventListener("dblclick",() => location.reload(true));
var turn = 0; 
const boxes = [...document.getElementsByClassName("box")];
const PLAYER_X = "X", PLAYER_O = "O";
const winPatterns = "012,345,678,036,147,258,048,246".split(",");

function popUpWindow(winner) {
    modalContainer.classList.add("display-modal"); // add CSS rule to show modal
    if (winner === "tie") { playerResult.textContent = "It's a Tie" }
    else {
        playerResult.textContent = "wins";
        playerResult.classList.add("modal-win-player-" + winner); 
    }
}
function winCheck() {
    for (const pat of winPatterns) {
        if (boxes[pat[0]].player && 
        boxes[pat[0]].player === boxes[pat[1]].player && 
        boxes[pat[1]].player === boxes[pat[2]].player) {
            popUpWindow(boxes[pat[0]].player);
            return;
        }
    }
    if (!boxes.some(box => !box.player)) { popUpWindow("tie") }
}
function turnCheck(event) {
    const box = event.target;
    if (box.classList.contains('box') && !box.player) {
        box.player = turn % 2 ? PLAYER_X : PLAYER_O;
        box.classList.add("board-moved-player-" + (turn++ % 2 ? PLAYER_X : PLAYER_O));
        winCheck();
    }
}

HTML

<div class="boundary" id="boundary">
  <div class="board" id="board" style="transform:rotate(180deg)">
    <div class="box"></div>
    <div class="box"></div>
    <div class="box"></div>
    <div class="box"></div>
    <div class="box"></div>
    <div class="box"></div>
    <div class="box"></div>
    <div class="box"></div>
    <div class="box"></div>
  </div>
</div>
<div id="modalContainer">
  <div class="customModal">
    <h2 class="player-won" id="playerResult"></h2>
    <button id="reloadButton">Play Again</button>
  </div>
</div>

Additional references and notes (*)

  1. Web API Element.id

  2. Polymorphism

  3. Ad hoc polymorphism

  4. There is still debate if direct element reference is good practice (a hang on from the 90s browser wars between Netscape and Microsoft)

    One argument against it is that support is not guaranteed. This only applies to older versions (~14) of FireFox which in fact does support direct reference, however its behavior in regard to non unique IDs is not the same as all other browsers.

    All browsers now conform to the annexed standard, referencing the first instance of an element in the case there are duplicated ID (Note that duplicated IDs will force the page into quirks mode)

    The other common argument is that it is not in the standard an thus support may be dropped at any time. This is untrue, support is annexed in the HTML5 spec (named access on window object) and is not going to disappear.

\$\endgroup\$
  • \$\begingroup\$ Thank you so much for such a detailed answer. I will keep in mind these points while coding next project. will go through the references thoroughly. Oh, and one more thing, while I rotated the board, I had in mind of crating a bot opponent, forgot to remove that, sorry. \$\endgroup\$ – Towkir Apr 15 at 16:26
  • 1
    \$\begingroup\$ This is a good and very comprehensive answer, which must have taken quite a lot of time to write. Kudos for that. It therefore came as a surprise to me that the given JS code doesn't actually work. It contains simple errors. Isn't is good practice to test your code before you publish it? I always do, because I invariably make stupid mistakes if I don't. \$\endgroup\$ – KIKO Software Apr 16 at 6:06
  • \$\begingroup\$ @KIKOSoftware Thanks for the heads up. All I could spot was PLAYER_Y rather than PLAYER_O I hope that was what you saw.. \$\endgroup\$ – Blindman67 Apr 16 at 6:56
  • \$\begingroup\$ Only one other thing: The variables board and reloadButton are not defined. That's easy to correct, and then it seems to work. Didn't test it thoroughly though. \$\endgroup\$ – KIKO Software Apr 16 at 7:41
  • \$\begingroup\$ @KIKOSoftware Both are defined in the HTML that is why I added the cutdown HTML to the answer. They join modalContainer playerResult as direct element references. (AKA named element access) \$\endgroup\$ – Blindman67 Apr 16 at 9:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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