6
\$\begingroup\$

I'm a beginner and just finished my first Tic Tac Toe program. I was looking to get some feedback on some of the portions that seem a bit repetitive. Is there a way to make this more dry vs. hard coding?

///////TicTacToe///////////

//////////BOARD ASSIGNMENT////////////
var a = "a";
var b = "b";
var c = "c";
var d = "d";
var e = "e";
var f = "f";
var g = "g";
var h = "h";
var i = "i";

play();

function buildBoard() {
    var board = "=======" + "\n" + a + "|" + b + "|" + c +"\n" +
    d + "|" + e + "|" + f + "\n" +
    g + "|" +  h + "|" + i + "\n" + "=======";
    console.log(board);
    return board;
    }

/////////PLAYER INPUT///////////
function getInput(){
    console.log("You are X, the computer is O" + "\n" +
        "X goes first");
    return prompt("You are X, the computer is O" + "\n" +
        "X goes first");
}
//////////CPU MOVE GENERATOR & VALIDATOR///////////
function randomNumber() {
    var random = Math.random();
    if (random > 0.89) {
        if (a !== 'X' && a !== 'O') {
        a = "O";
        return a;
        } else {return randomNumber();}
    } else if (random > 0.78) {
        if (b !== 'X' && b !== 'O') {
        b = "O";
        return b;
        } else {return randomNumber();}
    } else if (random > 0.67) {
        if (c !== 'X' && c !== 'O') {
        c = "O";
        return c;
        } else {return randomNumber();}
    } else if (random > 0.56) {
        if (d !== 'X' && d !== 'O') {
        d = "O";
        return d;
        } else {return randomNumber();}
    } else if (random > 0.45) {
        if (e !== 'X' && e !== 'O') {
        e = "O";
        return e;
        } else {return randomNumber();}
    } else if (random > 0.34) {
        if (f !== 'X' && f !== 'O') {
        f = "O";
        return f;
        } else {return randomNumber();}
    } else if (random> 0.23) {
        if (g !== 'X' && g !== 'O') {
        g = "O";
        return g;
        } else {return randomNumber();}
    } else if (random > 0.12) {
        if (h !== 'X' && h !== 'O') {
        h = "O";
        return h;
        } else {return randomNumber();}
    } else if (random > 0.00) {
        if (i !== 'X' && i !== 'O') {
        i = "O";
        return i;
        } else {
            return randomNumber();
        }
    }
}

//////////PLAYER MOVE VALIDATOR///////////
function playerMove() {
    var player = getInput();
        if (player === a && a !== 'X') {
            a = 'X';
        } else if (player === b && b !== 'X') {
            b = 'X';
        } else if (player === c && c !== 'X') {
            c = 'X';
        } else if (player === d && d !== 'X') {
            d = 'X';
        } else if (player === e && e !== 'X') {
            e = 'X';
        } else if (player === f && f !== 'X') {
            f = 'X';
        } else if (player === g && g !== 'X') {
            g = 'X';
        } else if (player === h && h !== 'X') {
            h = 'X';
        } else if (player === i && i !== 'X') {
            i = 'X';
        } else {
            console.log("=======" + "\n" + "Position already occupied" + "\n" + "Please Select Again" + "\n" + "=======");
            buildBoard();
            playerMove();
        }
}

function compare() {
    var p = playerMove();
    var cpu = randomNumber();
}


//////MAIN///////
function play() {
    buildBoard();
    compare();

    if (winner('X')) {
        buildBoard();
        console.log('You win!');
        return 'X';
    } else if (winner('O')) {
        buildBoard();
        console.log('The computer wins!');
        return 'O';
    } else {
        play();
    }
}

/////////DETERMINE WINNER IF WINNER FUNCTION STACK IS TRUE/////////////

////////TOP OF WINNER STACK////////////
function winner(player) {
    return winsRow(player) || winsColumn(player) || winsDiagonal(player);   
}

function winsRow(player) {
    return allThree(player, a, b, c) ||
           allThree(player, d, e, f) ||
           allThree(player, g, h ,i);
}

function winsColumn(player) {
    return allThree(player, a, d, g) ||
           allThree(player, b, e, h) ||
           allThree(player, c, f, i);
}
function winsDiagonal(player) {
    return allThree(player, a, e, i) ||
           allThree(player, c, e, g);
}
function allThree(player, cell_one, cell_two, cell_three) {
    return (cell_one === player) && (cell_two === player) && (cell_three === player);
}
/////////BOTTOM OF WINNER STACK///////////
\$\endgroup\$
2
  • 1
    \$\begingroup\$ At the start the board, the board assignment is redundant. \$\endgroup\$ – Evan Carslake Apr 10 '15 at 20:42
  • \$\begingroup\$ @EvanCarslake Please write all suggestions for improvements as answers, not comments. \$\endgroup\$ – 200_success Apr 11 '15 at 4:11
3
\$\begingroup\$

Rather than using individual variables to represent the cells in the game, it makes more sense to use a data structure like an array to represent the entire board. This also allows you to simple index into the array rather than having to make a giant switch/if statement in the player and cpu move functions.

Additionally, by keeping the labels separate from the board, we can simply check if a board cell is empty by !board[index] rather than having to check that it is not 'X' and not 'O': (cell !== 'X' && cell !== 'O').

Storing the labels in a string allows us to index in there on the player's input to get a board index easily: boardLabels.indexOf(prompt(msg)).

Full code below:

///////TicTacToe///////////

//////////BOARD ASSIGNMENT////////////
var board = [];
var boardLabels = 'abcdefghi';

play();

function buildBoard() {
    var boardString = "=======" + "\n";
    for(var i = 0; i < 9; i += 3) {
        boardString += (board[i] || boardLabels[i]) + "|";
        boardString += (board[i+1] || boardLabels[i+1]) + "|";
        boardString += (board[i+2] || boardLabels[i+2]) + "\n";
    }
    boardString += "=======";
    console.log(boardString);
    return board;
}

/////////PLAYER INPUT///////////
function getInput(){
    var msg = "You are X, the computer is O" + "\n" +
        "X goes first"
    console.log(msg);
    return boardLabels.indexOf(prompt(msg));
}

//////////CPU MOVE GENERATOR///////////
function getRandomNumber() {
    return Math.floor(Math.random()*9);
}

//////////CPU MOVE VALIDATOR///////////
function cpuMove() {
    var randomIndex = getRandomNumber();
    if(!board[randomIndex]) {
        board[randomIndex] = "O";
    } else {
        cpuMove();
    }
}

//////////PLAYER MOVE VALIDATOR///////////
function playerMove() {
    var chosenIndex = getInput();
    if(!board[chosenIndex]) {
        board[chosenIndex] = 'X';
    } else {
        console.log("=======" + "\n" + "Position already occupied" + "\n" + "Please Select Again" + "\n" + "=======");
        buildBoard();
        playerMove();
    }
}

function compare() {
    playerMove();
    cpuMove();
}


//////MAIN///////
function play() {
    buildBoard();
    compare();

    if (winner('X')) {
        buildBoard();
        console.log('You win!');
        return 'X';
    } else if (winner('O')) {
        buildBoard();
        console.log('The computer wins!');
        return 'O';
    } else {
        play();
    }
}

/////////DETERMINE WINNER IF WINNER FUNCTION STACK IS TRUE/////////////

////////TOP OF WINNER STACK////////////
function winner(player) {
    return winsRow(player) || winsColumn(player) || winsDiagonal(player);   
}

function winsRow(player) {
    return allThree(player, 0, 1, 2) ||
           allThree(player, 3, 4, 5) ||
           allThree(player, 6, 7 ,8);
}

function winsColumn(player) {
    return allThree(player, 0, 3, 6) ||
           allThree(player, 1, 4, 7) ||
           allThree(player, 2, 5, 8);
}
function winsDiagonal(player) {
    return allThree(player, 0, 4, 8) ||
           allThree(player, 2, 4, 6);
}
function allThree(player, cell_one, cell_two, cell_three) {
    return (board[cell_one] === player) && (board[cell_two] === player) && (board[cell_three] === player);
}
/////////BOTTOM OF WINNER STACK///////////
\$\endgroup\$
0
\$\begingroup\$

Using an array (as the previous answer mentions) will also cut a lot of hard-coding and repetitive typing out of your "winner" functions. It requires another variable at the start of your program:

var board = [0, 0, 0, 0, 0, 0, 0, 0, 0];
var labels = 'abcdefghi';
var wins = [
  [0,1,2],
  [3,4,5],
  [6,7,8],
  [0,3,6],
  [1,4,7],
  [2,5,8],
  [0,4,8],
  [2,4,6]
 ];

You can now use a loop in a single isWinner function which checks to see if one player occupies the three squares for a win condition. It has the added value of returning the winner 'x' or 'o' if there is one:

function isWinner(board){
  for(var i=0; i<this.wins.length; i++){
    var a, b, c;
    //these varaibles become what the board is holding 'x', 'o', or 0
    a = board[wins[i][0]];
    b = board[wins[i][1]];
    c = board[wins[i][2]];

    if(a == b && a == c && a != 0){
      return a;
    }
  }
  return false;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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