HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Tic-Tac-Toe code in C++

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
taccodetictoe

Problem

I wrote this as an exercise. I think that I have some repetitive code in the "winner" function but I don't know exactly how to reduce it. Any other general advice would be welcome too.

#include
#include

char table[3][3];
int turn=1;

void print_table()
{
    for (int i=0;i>x>>y;
    if(turn%2==1)
        table[x][y]='X';
    else
        table[x][y]='O';
}

char winner()
{
    for(int i=0;i<3;i++)
        if(table[0][i]==table[1][i]&&table[1][i]==table[2][i]&&table[0][i]=='X')
        {
            return 'X';
        }

    for(int i=0;i<3;i++)
        if(table[i][0]==table[i][1]&&table[i][1]==table[i][2]&&table[i][0]=='X')
        {
            return 'X';
        }

        for(int i=0;i<3;i++)
        if(table[0][i]==table[1][i]&&table[1][i]==table[2][i]&&table[0][i]=='O')
        {
            return 'O';
        }

    for(int i=0;i<3;i++)
        if(table[i][0]==table[i][1]&&table[i][1]==table[i][2]&&table[i][0]=='O')
        {
            return 'O';
        }

      if(table[0][0]==table[1][1]&&table[0][0]==table[2][2]&&table[0][0]=='O')
      {
            return 'O';
      }

      if(table[0][0]==table[1][1]&&table[0][0]==table[2][2]&&table[0][0]=='X')
      {
            return 'X';
      }

      return '\0'; ///default
}

int main(){

create_table();
print_table();

while (turn<=9&&winner()=='\0')
{
input(turn);
print_table();
winner();
turn++;
}

if(winner()!='\0')
    std::cout<<std::endl<<"winner:"<<winner();
else
    std::cout<<"Draw.";
}

Solution


  1. Simplified winner() method



You could merge the checks for 'X' and 'O' by returning one of the chars in a match of three in a row:

char winner() {
  for (int i = 0; i < 3; i++) {
    if (table[0][i] == table[1][i] && table[1][i] == table[2][i]) {
      return table[0][i]; // 'X' if all are 'X' and 'O' if all are 'O'
    }
  }

  for (int i = 0; i < 3; i++) {
    if (table[i][0] == table[i][1] && table[i][1] == table[i][2]) {
      return table[i][0];
    }
  }

  if (table[0][0] == table[1][1] && table[1][1] == table[2][2]) {
    return table[1][1];
  }

  if (table[0][2] == table[1][1] && table[1][1] == table[2][0]) { // *
    return table[1][1];
  }

  return '\0';
}


*Note that your code is missing the forward slash diagonal

  1. Consider eliminating your global variables



I know this is just a practice project, and your code is fine as it is. However, if you wanted to scale your code, then you've created some unnecessary problems for yourself in the future.

All of your functions depend on your two global variables, which limits you to one game and gets in the way of other features by polluting the global namespace. You could eliminate the globals by taking an object oriented approach or a more functional programming styled approach.

The Object Oriented way

The OO way would involve creating a TicTacToeGame class and making the global variables member variables members of your class.

class TicTacToeGame() {
private:
  char table[3][3];
  int turn;

public:  
  TicTacToeGame() {
    for (int i=0;i<3;i++)
      for (int j=0;j<3;j++)
        table[i][j]='*';

    turn = 1;
  }

  char winner() {}

  void input() {
    // ...
    turn++;
  }

  void print() {}

  int getTurn() {
    return turn;
  }
}

int main() {

  TicTacToeGame game = TicTacToeGame();
  game.print();

  while (game.getTurn() <= 9 && game.winner() == '\0')
  {
    game.input();
    game.print();
    game.winner();
  }

  if(game.winner() != '\0')
    std::cout<<std::endl<<"winner:"<<winner();
  else
    std::cout<<"Draw.";
}


Creating a class comes with a few other benefits. Particularly that you can encapsulate how a Tic-tac-toe game is played which helps with code scalability.

A more functional approach

For the functional approach you can just pass the globals as parameters instead (be sure to pass by reference):

char[][] create_table() {
  // ...
  return table;
}

void input (int turn, char[][] &table) {
  // ...
}

char winner(char[][] &table) {
  // ...
}

int main() {

  int turn = 1;
  char[][] table = create_table();
  print_table(table);

  while (turn <= 9 && winner(table) == '\0') {
    input(turn, table);
    print_table(table);
    winner(table);
    turn++;
  }

  if(winner(table) != '\0')
    std::cout<<std::endl<<"winner:"<<winner(table);
  else
    std::cout<<"Draw.";
}


This way gives you a lot more control over when and where turn and board get declared. You can more easily create multiple games to be played simultaneously. The biggest benefit is that your functions have their dependencies explicitly defined; there are no hidden parameters.

Nice job overall! My second suggestion is mostly just to help with a few concepts with software development, and it isn't really applicable if you intend to keep your Tic-tac-toe game as simple as it is. Keep up the good work.

Code Snippets

char winner() {
  for (int i = 0; i < 3; i++) {
    if (table[0][i] == table[1][i] && table[1][i] == table[2][i]) {
      return table[0][i]; // 'X' if all are 'X' and 'O' if all are 'O'
    }
  }

  for (int i = 0; i < 3; i++) {
    if (table[i][0] == table[i][1] && table[i][1] == table[i][2]) {
      return table[i][0];
    }
  }

  if (table[0][0] == table[1][1] && table[1][1] == table[2][2]) {
    return table[1][1];
  }

  if (table[0][2] == table[1][1] && table[1][1] == table[2][0]) { // *
    return table[1][1];
  }

  return '\0';
}
class TicTacToeGame() {
private:
  char table[3][3];
  int turn;

public:  
  TicTacToeGame() {
    for (int i=0;i<3;i++)
      for (int j=0;j<3;j++)
        table[i][j]='*';

    turn = 1;
  }

  char winner() {}

  void input() {
    // ...
    turn++;
  }

  void print() {}

  int getTurn() {
    return turn;
  }
}

int main() {

  TicTacToeGame game = TicTacToeGame();
  game.print();

  while (game.getTurn() <= 9 && game.winner() == '\0')
  {
    game.input();
    game.print();
    game.winner();
  }

  if(game.winner() != '\0')
    std::cout<<std::endl<<"winner:"<<winner();
  else
    std::cout<<"Draw.";
}
char[][] create_table() {
  // ...
  return table;
}

void input (int turn, char[][] &table) {
  // ...
}

char winner(char[][] &table) {
  // ...
}

int main() {

  int turn = 1;
  char[][] table = create_table();
  print_table(table);

  while (turn <= 9 && winner(table) == '\0') {
    input(turn, table);
    print_table(table);
    winner(table);
    turn++;
  }

  if(winner(table) != '\0')
    std::cout<<std::endl<<"winner:"<<winner(table);
  else
    std::cout<<"Draw.";
}

Context

StackExchange Code Review Q#141670, answer score: 2

Revisions (0)

No revisions yet.