\$\begingroup\$
\$\endgroup\$
2
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>-
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\$Perry– Perry2020年06月15日 22:15:36 +00:00Commented Jun 15, 2020 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– greybeard2020年06月16日 03:43:55 +00:00Commented Jun 16, 2020 at 3:43
1 Answer 1
\$\begingroup\$
\$\endgroup\$
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.
answered Jul 13, 2020 at 4:26
lang-css