current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 5 down vote favorite

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///////////
share|improve this question
1  
At the start the board, the board assignment is redundant. – Evan Carslake Apr 10 '15 at 20:42
    
@EvanCarslake Please write all suggestions for improvements as answers, not comments. – 200_success♦ Apr 11 '15 at 4:11
up vote 3 down vote

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///////////
share|improve this answer
up vote 0 down vote

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;
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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