3
\$\begingroup\$

Coming from a python background, I recently started learning django, currently at the javascript section. This tic-tac-toe code is to familiarize myself with javascript basic concepts. No application code is included, just JS and HTML.

tic-tac-toe.css

td {
    background: black;
    width: 2em;
    height: 2em;
    font-size: 3em;
    text-align: center;
    color: orange;
}

table {
    margin: auto;
}

h1 {
    margin-left: 46%;
}

tic-tac-toe.js

function markCell(cell, marks) {
    if (marks.length === 0) {
        marks.push('O')
        marks.push('X')
    }
    if (cell.textContent === '') {
        cell.textContent = marks.pop()
    }
}

function clearBoard() {
    for (let cell of document.getElementsByTagName('td')) {
        cell.textContent = ''
    }
}

function updateWinnerScore(winner) {
    let scores = document.querySelector('h1').innerText
    let score1 = parseInt(scores[0])
    let score2 = parseInt(scores[4])
    if (winner === 'XXX') {
        score1 += 1
        clearBoard()
    } else if (winner === 'OOO') {
        score2 += 1
        clearBoard()
    }
    document.getElementsByTagName('h1')[0].innerHTML = score1 + ' - ' + score2
}

function updateScores(cells) {
    let values = []
    for (let c of cells) {
        values.push(c.textContent)
    }
    let combinations = [values.slice(0, 3).join(''), values.slice(3, 6).join(''), values.slice(6, 9).join('')]
    for (let i = 0; i < 3; ++i) {
        combinations.push([values[i], values[i + 3], values[i + 6]].join(''))
    }
    combinations.push([values[0], values[4], values[8]].join(''))
    combinations.push([values[2], values[4], values[6]].join(''))
    let winner
    for (let state of ['XXX', 'OOO']) {
        if (combinations.includes(state)) {
            winner = state
        }
    }
    updateWinnerScore(winner)
    if (values.join('').length === 9) {
        clearBoard()
    }
}

function updateBoard(marks) {
    let cells = Array.from(document.getElementsByTagName('td'))
    for (let c of cells) {
        c.addEventListener('click', function () {
            markCell(c, marks)
        })
        c.addEventListener('click', function () {
            updateScores(cells)
        })
    }
}

tic-tac-toe.html

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <title>Tic Tac Toe</title>
    <link rel="stylesheet" href="tic-tac-toe.css">
    <script src="tic-tac-toe.js"></script>
</head>
<body>
<h1>0 - 0</h1>
<table>
    <tr>
        <td></td>
        <td></td>
        <td></td>
    </tr>
    <tr>
        <td></td>
        <td></td>
        <td></td>
    </tr>
    <tr>
        <td></td>
        <td></td>
        <td></td>
    </tr>
</table>
<script>
    let marks = ['O', 'X']
    updateBoard(marks)
</script>
</body>
</html>
\$\endgroup\$
2
  • \$\begingroup\$ What feedback are you looking for? \$\endgroup\$
    – Dave Meehan
    Commented Jul 18, 2022 at 14:55
  • \$\begingroup\$ conventions, anything that can be improved, simplified and whatnot. \$\endgroup\$
    – watch-this
    Commented Jul 18, 2022 at 15:31

1 Answer 1

Reset to default
1
\$\begingroup\$

There are many ways to implement TicTacToe, and its good to try out several of them. There are lots of examples on this site, it would serve you well to compare some of them and learn from the feedback given. The definition of 'improve' is rather subjective, depending on the problem at hand and the developer knowledge. Javascript is very flexible in terms of how you might organise the code.

I don't think there is much merit in actually outlining many of the ways you could refactor the code, so I'll just present a rework of the code that largely follows the existing, and some general pointers:

  1. Consider how functions and variables in the main scope affect the global scope. Reducing the number of global symbols will help you be a good citizen alongside other scripts. In my rework I've not gone all the way, just providing a hint that its easy to remove dependencies (such as on DOM element retrieval) and pass them in where necessary.

  2. You are largely maintaining game state in the DOM (for example using element.textContent). In a trivial example such as this that's likely ok, but as your script gain complexity, you'll want to consider maintaining state within the game and updating the DOM to match it. This will make debugging easier, as you can track state updates more easily. It's a technique used by many of the UI frameworks, so worth learning. I've gone some of the way, but not all, just to indicate how you can encapsulate some of the state into the entry point.

  3. In updateScores you had a somewhat complicated and repetitive way of determining which cells contributed to a win. Refactoring this out to a list of possibles and then comparing makes things clearer.

  4. Use functions (often arrow functions) to help describe what the purpose of a more complex statement is. You can see this with gameOver(). Or assign the result to a variable and then compare if you prefer, either way, the name of the variable/function makes the intention clearer.

function markCell(cell, player) {
    if (cell.textContent === '') {
        cell.textContent = player
        return true
    }
    return false
}

function updateScores(cells, reset, updateWinnerScore) {
    let values = cells.map(c => c.textContent)

    const lines = [
        [ 0, 1, 2 ],
        [ 3, 4, 5 ],
        [ 6, 7, 8 ],
        [ 0, 3, 6 ],
        [ 1, 4, 7 ],
        [ 2, 5, 8 ],
        [ 0, 4, 8 ],
        [ 2, 4, 6 ],
    ]
    
    const winner = lines.reduce((acc, line) => {
        return acc || line.reduce((acc, index) => { 
            return acc === values[index] ? acc : undefined 
        }, values[0])
    }, undefined)
    
    updateWinnerScore(winner)

    const gameOver = () => winner || values.join('').length == 9
    if (gameOver()) {
        reset()
    }
}

function startGame(container) {

    const cells = Array.from(document.querySelectorAll(`${container} td`))
    const heading = document.querySelector(`${container} .score`)
    let player
    
    if (cells.length !== 9 || heading === undefined) {
        alert('Invalid markup');
        return
    }
    
    const reset = () => {
        cells.forEach(c => c.textContent = '')
        player = 'X'
    }

    const updateWinnerScore = winner => {
        if (winner !== 'X' && winner !== 'O') {
            return 
        }
        const score = heading.querySelector(`.${winner}`)
        score.innerText = parseInt(score.innerText) + 1
    }
    
    reset()
    
    for (let c of cells) {
        c.addEventListener('click', function () {
            if (markCell(c, player)) {
                player = player === 'X' ? 'O' : 'X'
                updateScores(cells, reset, updateWinnerScore)
            }
        })
    }
}

startGame('.board')
td {
    background: black;
    width: 2em;
    height: 2em;
    font-size: 3em;
    text-align: center;
    color: orange;
}

table {
    margin: auto;
}

h1 {
    margin-left: 46%;
}
<div class="board">
    <h1 class="score"><span class="X">0</span> - <span class="O">0</span></h1>
    <table>
        <tr>
            <td></td>
            <td></td>
            <td></td>
        </tr>
        <tr>
            <td></td>
            <td></td>
            <td></td>
        </tr>
        <tr>
            <td></td>
            <td></td>
            <td></td>
        </tr>
    </table>
</div>

\$\endgroup\$

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