TicTacToe in Javascript

Question

I made this two player TicTacToe game as a challenge i was given in class. The game starts where the first player chooses where their X will be placed. Each area in the array has been given a number starting from 1 and ending at 9. After X chooses their area, the game will let the user know who's turn it is, and they will keep playing until one of them wins.

As of right now, the game doesn't let you know that nobody won, but that's another update i have to make.

Please do review the code and let me know how i can make it more efficient and i am also open for suggestions, so please be honest as much as possible as this is the first time i write in Javascript using Nodejs, and i would love to get better.

 //user input and format
let input = process.stdin;
input.setEncoding("utf-8");

console.log("You will start with X");
console.log("Please choose a box number from 1 to 9 to place your X");

//initializing an array of 9 fields where each one contains * as a start
let gameboard = []; // = ["*", "*", "*", "*", "*", "*", "*", "*", "*"];
let symbol = "*";

for (let i = 0; i < 9; i++) {
  gameboard[i] = symbol;
}

//determining who's turn it is and the players' symbols
let player1isplaying = true;
let player1 = "X";
let player2 = "O";

//Game loop
input.on("data", function(data) {
  console.clear();

  //bad input
  if (parseInt(data) > 9 || parseInt(data) < 1 || isNaN(data)) {
    console.log("Please enter a number between 1 and 9");
  } else if (gameboard[parseInt(data) - 1] != symbol) {
    console.log("The spot you're trying to fill is already taken.");
  }
  //which player's turn
  else if (player1isplaying) {
    gameboard[parseInt(data) - 1] = player1;
    player1isplaying = false;
  } else if (!player1isplaying) {
    gameboard[parseInt(data) - 1] = player2;
    player1isplaying = true;
  }

  showboard(gameboard);
  checkforXwin(gameboard);
  checkforOwin(gameboard);

  if (player1isplaying) {
    console.log(player1 + "'s turn");
  } else {
    console.log(player2 + "'s turn");
  }
});

//winner validation
function checkforXwin(array) {
  if (
    (array[0] == player1 && array[1] == player1 && array[2] == player1) ||
    (array[3] == player1 && array[4] == player1 && array[5] == player1) ||
    (array[6] == player1 && array[7] == player1 && array[8] == player1) ||
    (array[0] == player1 && array[4] == player1 && array[8] == player1) ||
    (array[2] == player1 && array[4] == player1 && array[6] == player1) ||
    (array[0] == player1 && array[3] == player1 && array[6] == player1) ||
    (array[1] == player1 && array[4] == player1 && array[7] == player1) ||
    (array[2] == player1 && array[5] == player1 && array[8] == player1)
  ) {
    console.log(player1 + " wins");
    process.exit();
  }
}

//winner validation
function checkforOwin(array) {
  if (
    (array[0] == player2 && array[1] == player2 && array[2] == player2) ||
    (array[3] == player2 && array[4] == player2 && array[5] == player2) ||
    (array[6] == player2 && array[7] == player2 && array[8] == player2) ||
    (array[0] == player2 && array[4] == player2 && array[8] == player2) ||
    (array[2] == player2 && array[4] == player2 && array[6] == player2) ||
    (array[0] == player2 && array[3] == player2 && array[6] == player2) ||
    (array[1] == player2 && array[4] == player2 && array[7] == player2) ||
    (array[2] == player2 && array[5] == player2 && array[8] == player2)
  ) {
    console.log(player2 + " wins");
    process.exit();
  }
}

//showing the board in 3 rows and 3 columns
function showboard(array) {
  console.log("---------");
  console.log(array.slice(0, 3).join("   "));
  console.log("");
  console.log(array.slice(3, 6).join("   "));
  console.log("");
  console.log(array.slice(6, 9).join("   "));
  console.log("---------");
}

Answer 1

First the really easy advice:

• use strict mode https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode
• use const instead of let for stuff you know you're not supposed to change (symbol, player1, player2); declaring it const helps you keep some assumptions later
• use === instead of == generally; I think the only legitimate use for == is for using it with null when you allow undefined or results of typeof if you feel like you're a JS human minifier :)
• prefer camel case since JS already does it and it would look weird to have two casing strategies in the same codebase
• use better naming in general: I'm talking about array in function checkforOwin(array) - array is extremely generic (what array?!?) I would call it board so I know what I'm dealing with; the fact that it's implemented as an array is a detail
• I'd also like a function named checkforOwin to just check and return true/false than exit the process xD
• who's O in this same function name? I thought we're comparing with the variable player1; stick to one or the other, not both; you'd have to update quite a bit of code if you change the initial statement of let player1 = "X"

...and the more involved:

• I would use nested arrays for the board because I can't easily picture what board[7] is; but I can picture what board[2][1] is
• those really long if statements look very repetitive; they look a lot like patterns on a board so instead let's turn that code into data so we can manage it easier; we can rewrite the patterns like this:

const isOWinner = [
  [0, 1, 2],
  [3, 4, 5],
  [6, 7, 8],
  [0, 4, 8],
// ...
].some((indices) => indices.every((index) => board[index] === player1))

That big condition blob in your if statements can be rewritten like this. Because it's an array you can store it somewhere; load it; merge with other patterns or extend or whatever you can do with arrays. Code that looks like data should be represented as data. In fact let's use the same data in both functions and merge them into one:

function checkForWin (board, player) {
  const isWinner = [
    [0, 1, 2],
    [3, 4, 5],
    [6, 7, 8],
    [0, 4, 8],
    // ...
  ].some((indices) => indices.every((index) => board[index] === player))

  if (isWinner) {
    console.log(player + 'wins')
    process.exit()
  }
}

And now you just call checkForWin(board, player1) or checkForWin(board, player2)

Answer 2

There is good advice in the answer by adrianton3. I noticed one other thing:

You may have already updated the structure of the board based on adrianton3's answer but instead of this:

let gameboard = []; // = ["*", "*", "*", "*", "*", "*", "*", "*", "*"];
let symbol = "*";

for (let i = 0; i < 9; i++) {
  gameboard[i] = symbol;
}

it could be simplified to use Array.fill():

const symbol = "*";
const gameboard = Array(9).fill(symbol);

That way there is no need to loop through the board when initializing the default values.

---

It likely won't be an issue when users input numbers 0-9 but something to be aware of is that calls to parseInt() without a radix parameter may yield unexpected results if the input happens to contain a leading zero (or else hex prefix: 0x).

If the radix is undefined, 0, or unspecified, JavaScript assumes the following:

1. If the input string begins with "0x" or "0X" (a zero followed by lowercase or uppercase X), radix is assumed to be 16 and the rest of the string is parsed as a hexidecimal number.
2. If the input string begins with "0" (a zero), radix is assumed to be 8 (octal) or 10 (decimal). Exactly which radix is chosen is implementation-dependent. ECMAScript 5 clarifies that 10 (decimal) should be used, but not all browsers support this yet. For this reason always specify a radix when using parseInt.
3. If the input string begins with any other value, the radix is 10 (decimal).¹

See this post for more information.

¹_{https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt#Description}

Answer 3

Extending on adrianton3's answer:

The following example not only reads better, but avoids re-creating the array of winning patterns every time you call the function:

const winningPatterns = [
  [0, 1, 2],
  [3, 4, 5],
  [6, 7, 8],
  [0, 4, 8],
  // ...
]
function checkForWin (board, player) {
  const isWinner = winningPatterns.some((indices) => indices.every((index) => board[index] === player))

  if (isWinner) {
    console.log(player + 'wins')
    process.exit()
  }
}