JavaScript Tic Tac Toe implementation

Question

I've been trying to get into web development, so I made a quick Tic Tac Toe implementation. It feels messy to me, but I'm not sure what is really considered to be good practice and what not. Any poor practice in my code, or things I could improve?

https://github.com/jason-shepherd/tictactoe

//Get the board, record, and select html elements
const gridSpaces = document.querySelectorAll('[data-spaces]');
const recordText = document.querySelector('[data-record]');
const difficultySelect = document.querySelector('[data-select]')

const gridWidth = Math.sqrt(gridSpaces.length);

let opponent = "O"
let player = "X"

let difficulty;
let record = {
    X: 0,
    O: 0,
    ties: 0
}
let moveCount = 0;
let inPlay = true;

function init() {
    updateDifficulty();
    //Init the board spaces with an event listener
    for(let i = 0; i < gridSpaces.length; i++) {
        gridSpaces[i].addEventListener('click', () => {
            if(!inPlay) {
                reset();
                return;
            }
            if(getSpaceValue(i) != '') return;

            //Player's move
            setSpaceValue(i, player);
            gridSpaces[i].style.cursor = "default";
            win = getWin(Math.floor(i % gridWidth), Math.floor(i / gridWidth), player);
            displayWin(win, player);
            moveCount++;
            
            //AI move
            if(inPlay) {
                if(difficulty != 0)
                    makeAiMove();
                else 
                    player = player == "O" ? "X" : "O";
            }
        });
    }
}

function checkRowSpace(index, x, y, board) {
    return getGridSpace(index, y, board);
}

function checkColSpace(index, x, y, board) {
    return getGridSpace(x, index, board);
}

function checkDiagonal(index, x, y, board) {
    if(x == y)
        return getGridSpace(index, index, board);
    else
        return null;
}

function checkAntiDiagonal(index, x, y, board) {
    if(x + y == gridWidth - 1)
        return getGridSpace(index, gridWidth - 1 - index, board);
    else
        return null
}

const checkFunctions = [checkRowSpace, checkColSpace, checkDiagonal, checkAntiDiagonal];
function getWin(x, y, currentPlayer, board) {
    let winSequence = [];

    for(let i = 0; i < 4; i++) {
        for(let j = 0; j < gridWidth; j++) {
            let currentSpace = checkFunctions[i](j, x, y, board);
            
            if(board == undefined) {
                if(getSpaceValue(currentSpace) != currentPlayer) {
                    winSequence = [];
                    break;
                }
            } else if(currentSpace != currentPlayer) {
                winSequence = [];
                break;
            }

            winSequence.push(currentSpace);
            if(j == gridWidth - 1) {
                return winSequence;
            }
        }
    }

    if(moveCount == Math.pow(gridWidth, 2) - 1) {
        return gridSpaces; 
    }

    return winSequence;

}

function displayWin(win, currentPlayer) {
    if(win.length !== 0) {
        let condition = "win";

        if(win.length === gridSpaces.length) {
            record.ties++;
            condition = "draw";
        } else {
            record[currentPlayer]++;
        }
        recordText.textContent = `X ${record.X}-${record.ties}-${record.O} O`;

        win.forEach(space => {
            space.firstChild.classList.add(condition);
        });

        gridSpaces.forEach(space => {
            space.style.cursor = "pointer";
        });
        inPlay = false;
        return;
    }
}

function makeAiMove() {
    let bestVal = -11;
    let bestMove;
    let newBoard = [];

    gridSpaces.forEach(space => {
        newBoard.push(getSpaceValue(space));
    });
    
    let possibleMoves = getBoardChildren(newBoard, "O");
    if(difficulty != 9)
        possibleMoves.sort((a, b) => {return 0.5 - Math.random()})
    possibleMoves.forEach(child => {
        let value = minimax(child, difficulty, false);
        if(value > bestVal) {
            bestVal = value;
            bestMove = child;
        }
    });

    for(let i = 0; i < bestMove.length; i++) {
        if(getSpaceValue(i) != bestMove[i]) {
            setSpaceValue(i, 'O');
            let win = getWin(Math.floor(i % gridWidth), Math.floor(i / gridWidth), opponent);
            displayWin(win, opponent);
        }
    }
    moveCount++;
}

function minimax(board, depth, maximizingPlayer) {
    let score = scoreBoard(board, depth);
    if(depth == 0 || isTerminating(board) || score != 0)
        return score;
    if(maximizingPlayer) {
        let value = -10;
        getBoardChildren(board, opponent).forEach(child => {
            value = Math.max(value, minimax(child, depth - 1, false));
        });
        return value;
    } else {
        let value = 10;
        getBoardChildren(board, player).forEach(child => {
            value = Math.min(value, minimax(child, depth - 1, true));
        });
        return value;
    }
}

function getBoardChildren(board, currentPlayer) {
    let children = [];
    for(let i = 0; i < board.length; i++) {
        if(board[i] == '') {
            board[i] = currentPlayer;
            children.push([...board]);
            board[i] = '';
        }
    }
    return children;
}

function isTerminating(board) {
    for(let i = 0; i < board.length; i++) {
        if(board[i] == '')
            return false;
    }
    return true;
}

function scoreBoard(board, depth) {
    let currentPlayer = "O";
    for(let i = 0; i < 2; i++) {
        for(let j = 0; j < 3; j++) {
            if(getWin(j, j, currentPlayer, board).length == 3) {
                if(currentPlayer == "O")
                    return 10 - (difficulty - depth);
                else
                    return -10 + (difficulty - depth);
            }
        }
        currentPlayer = "X";
    }
    return 0;
}

function updateDifficulty() {
    if(difficultySelect.value != "friend") {
        switch(difficultySelect.value) {
            case "easy":
                difficulty = 1;
                break;
            case "medium":
                difficulty = 4;
                break;
            case "unbeatable":
                difficulty = 9;
                break;
        }
        if(player == "O") {
            player = "X";
            makeAiMove();
        }
    } else {
        difficulty = 0;
    }
}

function reset() {
    player = "X";
    moveCount = 0;
    inPlay = true;

    for(let i = 0; i < gridSpaces.length; i++) {
        gridSpaces[i].firstChild.classList.remove("win");
        gridSpaces[i].firstChild.classList.remove("draw");
        setSpaceValue(i, "");
    }
}

function getSpaceValue(x, y, board) {
    if(x == null)
        return;
    else if(typeof x === 'object')
        return x.firstChild.textContent;
    else if(y == undefined)
        return gridSpaces[x].firstChild.textContent;
    else
        return gridSpaces[y * gridWidth + x].firstChild.textContent;
}

function setSpaceValue(index, value) {
    gridSpaces[index].firstChild.textContent = value;
}

function getGridSpace(x, y, board) {
    if(board != undefined)
        return board[y * gridWidth + x];
    else
        return gridSpaces[y * gridWidth + x];
}

init();

body {
   background-color: #353A47;
}

.tictactoe-container {
    background-color: #353A47;
    width: 77vh;
    height: auto;
    position: absolute;
    left: 50%;
    top: 20%;
    transform: translate(-50%, -20%);
}

.grid-container {
    background-color: #2B303B;
    display: inline-grid;
    width: auto;
    height: auto;
    grid-gap: 1vh;
    grid-template-columns: repeat(3, 25vh);
    grid-template-rows: repeat(3, 25vh);
}

.grid-item {
    background-color: #353A47;
    color: #F0F7EE;
    display: flex;
    cursor: pointer;
    justify-content: center;
    align-items: center;
    font-family: 'Varela Round', sans-serif;
    font-weight: bold;
    font-size: 25vh;
}

.record {
    color: #F0F7EE;
    font-weight: bold;
    text-align: center;
    font-family: 'Varela Round', sans-serif;
    font-size: 10vh;
    white-space: nowrap;
    margin: 2vh auto;
}

.ai-select {
    color: #F0F7EE;
    background-color: #353A47;
    font-size: 3vh;
    width: 40%;
    height: 15%;
    margin: 1vh 30%;
}

.win {
    color: #4BB3FD;
    animation: shake 0.5s;
    animation-iteration-count: 3;
}

.draw {
    color: #FF312E;
    animation: shake 0.5s;
    animation-iteration-count: 6;
}

@keyframes shake {
    0% { transform: translate(1px, 1px) rotate(0deg); }
    10% { transform: translate(-1px, -2px) rotate(-1deg); }
    20% { transform: translate(-3px, 0px) rotate(1deg); }
    30% { transform: translate(3px, 2px) rotate(0deg); }
    40% { transform: translate(1px, -1px) rotate(1deg); }
    50% { transform: translate(-1px, 2px) rotate(-1deg); }
    60% { transform: translate(-3px, 1px) rotate(0deg); }
    70% { transform: translate(3px, 1px) rotate(-1deg); }
    80% { transform: translate(-1px, -1px) rotate(1deg); }
    90% { transform: translate(1px, 2px) rotate(0deg); }
    100% { transform: translate(1px, -2px) rotate(-1deg); }
}

.noselect {
    -webkit-touch-callout: none; /* iOS Safari */
    -webkit-user-select: none; /* Safari */
    -khtml-user-select: none; /* Konqueror HTML */
    -moz-user-select: none; /* Old versions of Firefox */
    -ms-user-select: none; /* Internet Explorer/Edge */
    user-select: none; /* Non-prefixed version, currently
       supported by Chrome, Edge, Opera and Firefox */
}

@media screen and (orientation:portrait) {
    .tictactoe-container {
        width: 77vw;
    }

    .grid-container {
        grid-gap: 1vw;
        grid-template-columns: repeat(3, 25vw);
        grid-template-rows: repeat(3, 25vw);
    }

    .grid-item {
        font-size: 25vw;
    }
    
    .record {
        font-size: 10vw;
    }

    .ai-select {
        font-size: 3vw;
    }
}

<!DOCTYPE html>
<html>
    <meta charset="UTF-8" name="viewport" content="width=device-width, initial-scale=1">
    <head>
        <title>Tic Tac Toe</title>
        <link rel="stylesheet" type = "text/css" href = "style.css">
        <link href="https://fonts.googleapis.com/css2?family=Varela+Round&display=swap" rel="stylesheet"> 
    </head>
    <body>
        <div class="tictactoe-container">
            <p data-record class="record noselect">X 0-0-0 O</p>
            <div class="grid-container noselect">
                <div data-spaces class="grid-item"><p></p></div>
                <div data-spaces class="grid-item"><p></p></div>
                <div data-spaces class="grid-item"><p></p></div>
                <div data-spaces class="grid-item"><p></p></div>
                <div data-spaces class="grid-item"><p></p></div>
                <div data-spaces class="grid-item"><p></p></div>
                <div data-spaces class="grid-item"><p></p></div>
                <div data-spaces class="grid-item"><p></p></div>
                <div data-spaces class="grid-item"><p></p></div>
            </div>
            <select data-select class="ai-select" onchange="updateDifficulty()">
                <option value="easy">Easy</option>
                <option value="medium" selected="selected">Medium</option>
                <option value="unbeatable">Unbeatable</option>
                <option value="friend">Play with a friend</option>
            </select>
        </div>
        <script src="tictactoe.js" defer></script>
    </body>
</html>

Answer 1

Here are some suggestions and a version that tries to illustrate most of these points.

1. Separation of concerns

There should be some separation between the code that handles (i) the display and (ii) the underlying game logic and representation of the board. People often use design patterns like model-view-controller.

A classic way of separating them would be to make a class, module or object for the display and another for the underlying game logic. The two objects only call each others' methods in limited, well-defined cases. If you don't want to use objects you can just use function names and comments to have a clearer demarcation between the display/event handling and game logic.

Instead, you are at various points using the html both to represent data as well as the display. For simple programs, this might work and even simplify the code, but it isn't great for your program because you have functions that you want to be able to handle both an 'imaginary' board and the actual board shown on the screen. For more complex games, it will get more and more complicated to work with the html representation of the board. If you allow every function to change or read the html directly, it becomes hard to track where a particular change is coming from. It also binds your program too closely to a particular representation. For example, imagine you wanted to use canvas or svg graphics instead of just text and css. This would require rewriting everything with the current set-up.

getWin is particularly confusing as it seems to work completely differently depending on whether you pass it the board parameter or not. You may be trying to optimize by only checking rows containing the particular x or y, but it is much easier to write a generic function that will check any board. The function returns a sequence which is either the winning sequence for a win, or the whole grid if it is a draw.

2. Small functions that do one thing

Your functions seem messy, partly because they are using a mixture of the on-screen representation of the board and a separate array representation to find whether a space is filled or not, but also because they are trying to work in too many different cases. e.g. getWin (as mentioned above) and getSpaceValue, which accepts values of x that are null, an html element, or a number, and y can also be undefined or a number. Most functions should accept inputs of a single type (an exception is sometimes allowing arguments to be omitted with defaults) and return a predictable return value.

3. Use modern Array methods more

filter, find, map can all help write shorter and more readable code (although can also become unreadable if used to pack too much into a one-liner)

4. Use utility functions

You may not want to use a library like lodash, but a few standard utility functions would make your code more concise and readable, and using a popular library means that other programmers will be able to read your code and quickly understand what it is doing. For example lodash's maxBy would be useful in choosing the optimal AI move. You can easily write your own if you don't want to use a library.

5. Code for determining a win

Your code works and is clever in a way but very convoluted! Sometimes you pass it the coords of the current move, but at one point you pass j, j as the coordinates, which mysteriously still works. A simpler and more transparent way to check for a win is to store an array of possible winning sequences ([[0, 1, 2], [3, 4, 5], ...]) and then find the first sequence for which all board squares are set to player.

const range = [...Array(gridWidth).keys()];
const lines = [
    ...range.map(i => range.map(j => i * gridWidth + j)),   // rows
    ...range.map(i => range.map(j => i + j * gridWidth)),   // columns
    range.map(j => j * (gridWidth + 1)),                // diagonal
    range.map(j => (gridWidth - 1) * (j + 1))           // antidiagonal
    ];
function win(player, board) {
    return lines.find(line => line.every(i => board[i] === player));
}

If you can provide the coordinates of the current move, then you can write a potentially more efficient version of this which checks only the (max.) 4 lines that pass through those coordinates:

const range = [...Array(gridWidth).keys()];
function win(player, x, y, board) {
    let lines = [
        range.map(i => y * gridWidth + i),      // current row
        range.map(i => i * gridWidth + x)       // current column
        ];
    if (x === y) {              // diagonal
        lines.push(range.map(i => i * gridWidth + i));
    }
    if (gridWidth - x === y) {  // antidiagonal
        lines.push(range.map(i => (gridWidth - 1) * (i + 1)));
    }
    return lines.find(line => line.every(i => board[i] === player));
}

In practice for a 3x3 board this optimisation is unlikely to improve speed (and might worsen it, as the lines array has to be rebuilt each time).

6. html event handlers

Don't put event handlers in the html when most of your logic is in javascript. It's too messy and confusing. Put all the event handling in javascript.

7. Comments / documentation

As you are sharing your code and asking for feedback, you should document your code. At the simplest this could be a line or two of comments explaining what each function does and the overall structure. But there is a standard way of documenting called jsdoc (see also this intro). This is especially important if you for some reason still need to have functions that work in surprising ways.

Answer 2

To get into best practices would require volumes and get you plenty of opinion. At any rate, trying to program too far beyond your understanding is no good so I won't get into particulars. In general, the first thing that occurs to me is that if you're really serious about programming, be good to your future self. Programming starts as a thought process and your thoughts will change as your skills grow. Leave liberal notes (code comments) and six months or six years from now when you ( or perhaps me!) revisit your code, you can reconstruct the process that made the code.

Kudos on naming your functions and variables so their intent is clear. Your indenting and coding style is consistent and that's important. Now get in the habit of leaving a line or two for each code block.

I've been programming for the web for a couple of decades and you won't believe the stress of trying to make a minor change to 600 lines of logic you don't understand when the company you're working for bills a million a year and employs dozens of people. Or the shock when you realize it's your own programming!