Avoiding nested loops in a TicTacToe algorithm JavaScript

Question

I wrote a quick algorithm that detects a win in a TicTacToe Game.

fn game() loops through a simulated ticTacToe board, finds all the index's of a given value, then loops through all the possible winning combos, finds any index's where the actual value is equal to the index's from the first loop, (I know :/) then replaces the value with the value passed into the function (either x or o).

Then, a subsequent function is run that checks if every value inside any of the nested arrays inside the possibleWinCombos are all equal to each other. If true, a winner is found.

const possibleWinCombos = [
  [0, 1, 2], //================ //
  [3, 4, 5],  //== Horizontal == //
  [6, 7, 8],  //=================//

  [0, 3, 6],  //================ //
  [1, 4, 7],  //== Vertical == //
  [2, 5, 8],  //=================//
  
  [0, 4, 8],  //=================//
  [2, 4, 6],  //== Diagonal == //
]

const ticTacToe = [
  'x', 'o', null,
  null, 'x', null,
  'o', null, 'x',
]

const game = (value) => {
  ticTacToe.forEach((cell, idxOfCell) => {
    if(cell === value){
      for (winCombo of possibleWinCombos){
        let idxToReplace = winCombo.indexOf(idxOfCell) 
        winCombo.splice(idxToReplace, 1, value)
      }     
    }
  })
  
  checkForWinner(value) ? 
    console.log(`${value}'s win`) : console.log('game still going')
}

const checkForWinner = (value) => {
  return possibleWinCombos.some((winCombo) => {
    return winCombo.every((winComboValue) => winComboValue === value);
  })  
}

game('x') // x's win
game('o') // game still going

It's only two functions so it's not very code heavy but I feel like game() wouldn't score very high on the performance rankings due to the nested looping. Any recommendations on some improvements would be great.

https://replit.com/@uwitdat/DrabNoteworthyLogin#script.js

Answer 1

You should either end your statements with semicolons consistently, or not at all. I recommend using semicolons consistently.

When writing for (winCombo of possibleWinCombos), you didn't declare winCombo, so it's a global variable.

The biggest problem with your code is that it trashes the contents of possibleWinCombos, such that after execution, it has this value:

possibleWinCombos = [
  ["x", "o", "o"],
  [  3, "x", "o"],
  ["o",   7, "o"],
  ["x",   3, "o"],
  ["o", "x", "o"],
  [  2,   5, "o"],
  ["x", "x", "o"],
  [  2, "x", "o"]
]

… even though it was ostensibly const. That means that if you ever run a rematch, your code won't work correctly unless you reinitialize possibleWinCombos. If you're writing code in a functional style, you should aim for your functions to have no side-effects.

You don't really have to worry about performance or scalability, since the board will always be no bigger than 3×3. That said, you should be aware that your algorithm calls .indexOf(), which involves a hidden loop to scan its array.

I recommend writing an isWinner(board, player) function. The two parameters make it clear what the inputs are. The name of the function makes it clear that it returns a boolean. By defining it using an IIFE, we can make it so that possibleWinCombos is available only to the code within isWinner(), which avoids polluting the global namespace.

const isWinner = (() => {
  const possibleWinCombos = [
    [0, 1, 2],  //================//
    [3, 4, 5],  //== Horizontal ==//
    [6, 7, 8],  //================//

    [0, 3, 6],  //================//
    [1, 4, 7],  //=== Vertical ===//
    [2, 5, 8],  //================//
  
    [0, 4, 8],  //================//
    [2, 4, 6],  //=== Diagonal ===//
  ];

  return (board, player) => {
    return possibleWinCombos.some((winCombo) => {
      return winCombo.every((i) => board[i] === player);
    });
  };
})();


const ticTacToe = [
  'x', 'o', null,
  null, 'x', null,
  'o', null, 'x',
];

if (isWinner(ticTacToe, 'x')) console.log('x won');
if (isWinner(ticTacToe, 'o')) console.log('o won');