8
\$\begingroup\$

This is my first web app written in JS. I would like to know how the code can be improved in terms of style and structure, especially in terms of following JS's conventions and general heuristics. I would also be thankful for any improvements that could be made to my HTML or CSS.

var size = 3;
var board = [];
var team = "X"
var running = true;

buildEmptyBoard();
addClickListenerToEachCell();

// Initialise empty board dynamically based on size
function buildEmptyBoard() {
    for (let i = 0; i < size; i++) {
        board.push([]);
        for (let j = 0; j < size; j++) 
            board[i].push(" ");     
    }
}

function addClickListenerToEachCell() {
    var matches = document.querySelectorAll("#board .row .elem .clickable");

    for (let i = 0; i < matches.length; i++) 
        matches[i].addEventListener("click", dealWithUserMove);
}

function dealWithUserMove(clickable) {
    var id = clickable.target.id;

    if (isValidMove(id))
        makeMove(id);
    else
        alert("Invalid move");

    simpleAIMove();
}


// A very simple AI that just picks the first possible move that's valid
function simpleAIMove() {
    if (running) {
        for (let i = 0; i < size; i++) {
            for (let j = 0; j < size; j++) {
                if (board[i][j] == " ") {
                    makeMove(convertCoordinatesToID(i, j));
                    return;
                }
            }
        }
    }
}

// Given 2 co-ordinates, convert to a string ID that the html will understand
function convertCoordinatesToID(i, j) {
    return "r" + i + "c" + j;
}


function isValidMove(id) {
    return board[getRowFromID(id)][getColumnFromID(id)] == " ";
}

function makeMove(id) {
    updateViewWithMove(id);
    updateBoardWithMove(id);

    detectGameOver(id);
    switchTeam();
}

// If game has finished, clean up and alert the appropriate message
function detectGameOver(id) {
    if (isWinner(id)) {
        alert("Team " + team + " has won!");
        running = false;
        removeClickListenerFromEachCell();
    }
    else if (isStalemate()) {
        alert("Game ended in statemate");
        running = false;
        removeClickListenerFromEachCell();
    }
}

function removeClickListenerFromEachCell() {
    var matches = document.querySelectorAll("#board .row .elem .clickable");

    for (let i = 0; i < matches.length; i++) 
        matches[i].removeEventListener("click", dealWithUserMove);
}

function isStalemate() {
    for (let i = 0; i < size; i++) {
        for (let j = 0; j < size; j++) {
            if (board[i][j] == " ") 
                return false;           
        }
    }

    return true;
}

// Return true if move resulted in a win
function isWinner(id) {
    return isSidewaysWinner(id) || isDiagonalWinner(id);
}

// Return true if move resulted in a vertical or horizontal win
function isSidewaysWinner(id) {
    var column = 0;
    var row = 0;

    for (let i = 0; i < size; i++) {
        if (board[getRowFromID(id)][i] == team)
            column++;
        if (board[i][getColumnFromID(id)] == team)
            row++;
    }

    return row == size || column == size;
}

function isDiagonalWinner(id) {
    posGradient = true;
    negGradient = true;

    for (let i = 1; i < size; i++) {
        if (board[i][i] != board[i - 1][i - 1] || board[i - 1][i - 1] == " ")
            posGradient = false;

        if (board[size - 1 - i][i] != board[size - i][i - 1] || board[size - i][i - 1] == " ")
            negGradient = false;
    }

    return posGradient || negGradient;
}

function switchTeam() {
    if (team == "X")
        team = "O"
    else 
        team = "X"
}

function updateBoardWithMove(id) {
    board[getRowFromID(id)][getColumnFromID(id)] = team;
}

function updateViewWithMove(id) {
    var cell = document.querySelector("#board .row .elem #" + id);
    cell.innerHTML = team;
    cell.style.color = "black";
}

function getRowFromID(id) {
    return parseInt(id.charAt(1));
}

function getColumnFromID(id) {
    return parseInt(id.charAt(3));
}
#page {
  width: 50%;
  margin: auto;
  font-family: Helvetica-Neue, sans-serif;
}

#board {
  text-align: center;
  border-collapse: collapse;
  border-style: hidden;
  pointer-events: none;
  table-layout: fixed
}

#board .row {
  pointer-events: none;
}

#board .row .elem {
  border: 5px solid black;
  font-size: 1000%;
  pointer-events: none;
}

#board .row .elem .clickable {
  pointer-events: all;
  color: white;
}
<!DOCTYPE html>
<html lang="en">

<head>
  <title>Tic Tac Toe</title>
  <meta charset="utfc8">
</head>

<body>
  <div id="page">
    <table id="board" height="600" width="600">
      <tr class="row">
        <td class="elem"><span class="clickable" id="r0c0">X</span></td>
        <td class="elem"><span class="clickable" id="r0c1">X</span></td>
        <td class="elem"><span class="clickable" id="r0c2">X</span></td>
      </tr>

      <tr class="row">
        <td class="elem"><span class="clickable" id="r1c0">X</span></td>
        <td class="elem"><span class="clickable" id="r1c1">X</span></td>
        <td class="elem"><span class="clickable" id="r1c2">X</span></td>
      </tr>

      <tr class="row">
        <td class="elem"><span class="clickable" id="r2c0">X</span></td>
        <td class="elem"><span class="clickable" id="r2c1">X</span></td>
        <td class="elem"><span class="clickable" id="r2c2">X</span></td>
      </tr>
    </table>
  </div>
</body>

</html>

\$\endgroup\$
2
  • 1
    \$\begingroup\$ @greybeard That's the way the snippet editor orders them. The actual markdown parser doesn't appear to care, so you could reorder them if you want to, but I'm not going to submit a functionally cosmetic suggested edit and waste reviewers' time. \$\endgroup\$ – pppery Jun 15 '20 at 22:15
  • \$\begingroup\$ (@pppery: as you can see, I left the decision to somebody wiser (or more judgemental). I remember trying to go easy on reviewers' time and appreciate the consideration.) \$\endgroup\$ – greybeard Jun 16 '20 at 3:43
4
\$\begingroup\$

Program looks well structured, so this is gonna be nitpicky

function isDiagonalWinner(id) {
    posGradient = true;

Since posGradient hasn't been declared anywhere, this will be treated as a global on window['posGradient']. let would suit you nicely here.

var matches = document.querySelectorAll("#board .row .elem .clickable");

The value of matches never changes. Using const here would indicate this.

var column = 0;

let is a more modern keyword for declaring variables that lives in block scope rather than function scope. For local vaiables in functions, it will likely more often be in line with what you want. Note that it doesn't make a difference here, but can when callbacks are involved.

\$\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.