2
\$\begingroup\$

I've created a Tic Tac Toe game in JS/HTML/CSS. Is there anyway this can be improved? Thanks

Codepen: https://codepen.io/KhushrajRathod/pen/wvaeOaZ

Github: https://github.com/KhushrajRathod/TicTacToe

Demo: https://www.khushrajrathod.me/TicTacToe

Javascript:

"use strict";

const grid = document.getElementById("grid").children

const x =
    `<svg viewBox="0 0 40 40">
    <path style="stroke: #506ded; stroke-width: 2;" d="M 10,10 L 30,30 M 30,10 L 10,30"></path>
</svg>`

const o =
    `<svg viewBox="0 0 40 40">
     <circle cx="20" cy="20" r="12" fill="#fff" style="stroke: #ea4335; stroke-width: 1.7;"></circle>
</svg>`

const winningPatterns = [
    // Horizontal
    [0, 1, 2],
    [3, 4, 5],
    [6, 7, 8],

    // Vertical
    [0, 3, 6],
    [1, 4, 7],
    [2, 5, 8],

    // Diagonal
    [0, 4, 8],
    [2, 4, 6]
]

let turn = x // Player 1 = X, Player 2 = Y
let turnsPlayed = 0

for (const item of grid) {
    item.addEventListener("click", () => {
        if (!item.innerHTML) { // Because "" is falsy
            item.innerHTML = turn
            turnsPlayed++
            checkWin()
            turn === x ? turn = o : turn = x // Reverse the value of "current"
        }
    })
}

function checkWin() {
    for (const pattern of winningPatterns) {
        if (grid[pattern[0]].innerHTML.toUpperCase() === turn.toUpperCase() && grid[pattern[1]].innerHTML.toUpperCase() === turn.toUpperCase() && grid[pattern[2]].innerHTML.toUpperCase() === turn.toUpperCase()) {
            if (turn === x) {
                win("X")
            } else {
                win("O")
            }
            return
        }
    }

    if (turnsPlayed === 9) {
        draw()
    }
}

function win(player) {
    swal({
        title: player + " wins!",
        text: "Would you like to play again?",
        icon: "success",
        buttons: ["No", "Yes"]
    }).then ((choice) => {
        if (!choice) { // NO
            // noinspection SillyAssignmentJS
            document.body.outerHTML = document.body.outerHTML // Disallow further moves by removing EventListeners
        } else {
            for (const item of grid) {
                item.innerHTML = ""
            }
            turnsPlayed = 0
        }
    })
}

function draw() {
    swal({
        title: "Draw!",
        text: "Would you like to play again?",
        icon: "info",
        buttons: ["No", "Yes"]
    }).then ((choice) => {
        if (!choice) { // NO
            // noinspection SillyAssignmentJS
            document.body.outerHTML = document.body.outerHTML // Disallow further moves by removing EventListeners
        } else {
            for (const item of grid) {
                item.innerHTML = ""
            }
            turnsPlayed = 0
        }
    })
}

HTML (excluding HTML5 Boilerplate):

<div id="container">
    <div id="grid">
        <div id="1"></div>
        <div id="2"></div>
        <div id="3"></div>

        <div id="4"></div>
        <div id="5"></div>
        <div id="6"></div>

        <div id="7"></div>
        <div id="8"></div>
        <div id="9"></div>
    </div>
</div>

CSS (excluding normalize and HTML5 Boilerplate):

body,
html {
    height: 100%;
    width: 100%;
    overflow: hidden;
}

#container {
    width: 100%;
    height: 100%;
    display: flex;
    align-items: center;
    justify-content: center;
}

#grid {
    border: 2px solid #808080;
    max-width: 400px;
    max-height: 400px;
    width: 100%;
    height: 100%;
    display: grid;
    grid-template-columns: 1fr 1fr 1fr;
    grid-template-rows: 1fr 1fr 1fr;
}

#grid > * {
    border: 1px solid #808080;
}

* {
    box-sizing: border-box;
}
/*  X or O */
#grid > * > * {
    width: 100%;
    height: 100%;
}

/*! HTML5 Boilerplate v7.3.0 | MIT License | https://html5boilerplate.com/ */
/* main.css 2.0.0 | MIT License | https://github.com/h5bp/main.css#readme */
/*
 * What follows is the result of much research on cross-browser styling.
 * Credit left inline and big thanks to Nicolas Gallagher, Jonathan Neal,
 * Kroc Camen, and the H5BP dev community and team.
 */

audio,
canvas,
iframe,
img,
svg,
video {
  vertical-align: middle;
}
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$
  1. if (!item.innerHTML) { // Because "" is falsy.

    If you need to add a comment then maybe the code isn't clear enough. I suggest either comparing it implicitly to an empty string, or using a data attribute to mark it as filled.

  2. In checkWin and the onClick listener, instead of relying on the innerHTML value, you should either use a virtual grid state, or save the state in a data attribute.

  3. The callback function of the would you like to play again is repeated for both the win and the draw functions. Thus, it should be extracted to be it's own function.

\$\endgroup\$
2
\$\begingroup\$
  1. I really don't like this svg code in javascript string. Why don't you put in html instead? It's presentation detail of your X and O has no meaning in your code. Edit: or even separate .svg file
  2. Extract your data from HTML. Your "data" are in html elements and innerHTML. That's just too tight.
  3. Your code is mix of different responsibilities (game itself, UI). Try to make tic tac toe game, which is completely independent of HTML. Good practice is to get it working using developer console. For example by adding function move(x,y). Then you add another piece of code, that will handle only your game HTML interface - for example move function will be called from event fired by click on element. That way your code doesn't mix too many things at once and is a lot more clear. Make more functions, make them return value, use that value (ex: checkWin should return maybe boolean or maybe player who won or undefined, some other function should update UI based on the result). You can also very simply unit test that kind of code.
\$\endgroup\$

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.