JavaScript code to check whether X won a tictactoe game

Question

My code is already doing what I want, so it's functional, now my concern is, if there is a way to make it easier to read or even better and shorter my code, it's supposed to check if 3 'X' or 'O' are continuous in 3 cells in a row, column or diagonal, code for the string 'O' is the same as 'X' only changes classList.contains('targetO'); else is the same. it's a tictactoe game.

    //horizontal X wins
if (td[0].classList.contains ('targetX') && td[1].classList.contains ('targetX') && td[2].classList.contains ('targetX')){ alert('X player Won')}
if (td[3].classList.contains ('targetX') && td[4].classList.contains ('targetX') && td[5].classList.contains ('targetX')){ alert('X player Won')};
if (td[6].classList.contains ('targetX') && td[7].classList.contains ('targetX') && td[8].classList.contains ('targetX')){ alert('X player Won')};

//vertical X wins
if (td[0].classList.contains ('targetX') && td[3].classList.contains ('targetX') && td[6].classList.contains ('targetX')){ alert('X player Won')}
if (td[1].classList.contains ('targetX') && td[4].classList.contains ('targetX') && td[7].classList.contains ('targetX')){ alert('X player Won')};
if (td[2].classList.contains ('targetX') && td[5].classList.contains ('targetX') && td[8].classList.contains ('targetX')){ alert('X player Won')};

//diagonal X wins

if (td[0].classList.contains ('targetX') && td[4].classList.contains ('targetX') && td[8].classList.contains ('targetX')){ alert('X player Won')}
if (td[2].classList.contains ('targetX') && td[4].classList.contains ('targetX') && td[6].classList.contains ('targetX')){ alert('X player Won')};

I made a fiddle: https://jsfiddle.net/ycedmfn2/

Answer 1

Dealing with verbosity

When we have lots of similar length statements, we can write helpers to replace the verbosity:

const cell = (index, symbol) => td[index].classList.contains(`target${symbol}`)

if (cell(0, 'X') && cell(1, 'X') && cell(2, 'X')) { alert('Player X won') }
// etc

Dealing with repetition

When we have statements that repeat with only variation in certain values, we can write loops or iterations:

if ([0, 1, 2].reduce((acc, cur) => acc && cell(cur, 'X'), true) { alert('Player X won') }
if ([3, 4, 5].reduce((acc, cur) => acc && cell(cur, 'X'), true) { alert('Player X won') }
// etc

Still kinda verbose and lacking in clarity, so:

const isWinLine = (indices, symbol) => indices.reduce((acc, cur) => acc && cell(cur, symbol), true)

if (isWinLine([0, 1, 2], 'X')) { alert('Player X won') }
if (isWinLine([3, 4, 5], 'X')) { alert('Player X won') }
// etc

We can remove the repetition of the alerts:

if (isWinLine([0, 1, 2], 'X') || isWinLine([3, 4, 5], 'X') /* etc */) {
  alert('Player X won')
}

And further:

const matrix = [
  [ 0, 1, 2 ], // R1
  [ 3, 4, 5 ], // R2
  [ 6, 7, 8 ], // R3
  [ 0, 3, 6 ], // C1
  [ 1, 4, 7 ], // C2
  [ 2, 5, 8 ], // C3
  [ 0, 4, 8 ], // Diag 1
  [ 6, 4, 2 ], // Diag 2
]

const isWinner = (symbol) => matrix.reduce((acc, indices) => acc && isWinLine(indices, symbol), true)

if (isWinner('X')) {
  alert('Player X won')
} else if (isWinner('O')) {
  alert('Player O won')
}

Querying CSS

As an alternative, it would be possible to use document.querySelectorAll that looks for specific classes and nth-child matches that correlate to rows, columns and diagonals, combined with whether the cell has a class to indicate who its been played by.

You'd just be looking for rows, columns or diagonals with 3 matching elements to confirm a win.

Answer 2

In software development, it's important to decouple the core logic from the technology you're using (easier to maintain, easier to test, etc).

What you're doing is mixing your core logic with HTML elements. They've become bound to each other and adding features may prove to be more complex. It would be much more clean to have your core logic only in javascript and then to link your code to your HTML elements (via event listeners, etc).

8 months ago, I quickly wrote a backend solution with denotypescript. You can check it out for inspiration.

Here are some tips.

1. You need at minimum 5 moves for there to be a winner, before that there is no need to check
2. The current player making a move is the only one that can win, so you only need to check if the curret player has a winning move.
3. You don't need to check for a diagonal win, if the spot being played isn't on a diagonal

Here is a snippet of a class method that checks if someone won:

    private findWinner(x: number, y: number, index: number) {

        // at least 5 plays must have been made
        // for there to be a winner
        if(this.playsCount < 5) {
            return false;
        }

        // get the current player (O or X)
        const player = this.board[index];

        // does the current player have a vertical win?
        const verticalWinner = [0, 1, 2].every(xd => player === this.board[(xd * 3 + y) % this.boardSize]);

        // does the current player have a horizontal win?
        const horizontalWinner = [0, 1, 2].every(yd => player === this.board[(x * 3 + yd) % this.boardSize]);

        if(verticalWinner || horizontalWinner) {
            return true;
        }

        // Is the current move part of a diagonal move?
        if((x + y) % 2 !== 0) {
            return false;
        }

        // if the current move is part of a diagonal move, did the current player win?
        const diagonal1Winner = [0, 1, 2].every(d => player === this.board[(d * 3 + d)]);
        const diagonal2Winner = [0, 1, 2].every(d => player === this.board[((2 - d) * 3 + d)]);
        return diagonal1Winner || diagonal2Winner;
    }