principlecMajor
Tic Tac Toe vs PC
Viewed 0 times
tactictoe
Problem
Please help me reduce its complexity and maybe optimize it a little bit more.
```
#include
#include
#include
#define size 3
void Clear_board(char (*)[size]);
void Player_turn(char (*)[size],char);
void PC_turn(char ()[size],char,char,int);
void Print_board(char (*)[size]);
void Scan_corners(char (*)[size],char,char);
int Check_board(char ()[size],char,int);
int Action(char (*)[size],char,char,int);
int Scan_diag(char (*)[size],char,char,int);
int Scan_rowscols(char (*)[size],char,char,int);
int main()
{
//-------------------------
char xo[size][size]={'1','2','3','4','5','6','7','8','9'};
char Fig_PC='X';
char Fig_Player='O';
int PC_move=0;
//-------------------------
printf("Welcom to Tic Tac Toe \n");
Print_board(xo);
Clear_board(xo);
//PC plays first and increments PC_move
PC_turn(xo,Fig_PC,Fig_Player,&PC_move);
//While nobody won, or PC reaches max move game is on
while ((Check_board(xo,Fig_PC,&PC_move)))
{
Player_turn(xo,Fig_Player);
PC_turn(xo,Fig_PC,Fig_Player,&PC_move);
}
return 0;
}
void PC_turn(char (xo)[size],char Fig_PC,char Fig_Player,int PC_move)
{
printf("Computer's Turn:\n");
/*
First try to win by completing an X X X
Second priority is to defend
Lastly try to take postion of corners
*/
if (!Action(xo,Fig_PC,Fig_Player,1))
{
if (!Action(xo,Fig_Player,Fig_PC,0))
{
Scan_corners(xo,Fig_PC,Fig_Player);
}
}
Print_board(xo);
++*PC_move;
}
void Print_board(char (*xo)[size])
{
int i=0;
printf("\n\n | | \n");
printf(" %c | %c | %c\n", xo[2][0], xo[2][1], xo[2][2]);
printf("___|___|___ \n");
printf(" | | \n");
printf(" %c | %c | %c\n", xo[1][0], xo[1][1], xo[1][2]);
printf("___|___|___ \n");
printf(" | | \n");
printf(" %c | %c | %c\n", xo[0][0], xo[0][1], xo[0][2]);
printf(" | | \n\n\n");
usleep(1000000);
}
void Clear_board(char (*xo)[size])
{
int i=0;
int j=0;
for (i=0;i
```
#include
#include
#include
#define size 3
void Clear_board(char (*)[size]);
void Player_turn(char (*)[size],char);
void PC_turn(char ()[size],char,char,int);
void Print_board(char (*)[size]);
void Scan_corners(char (*)[size],char,char);
int Check_board(char ()[size],char,int);
int Action(char (*)[size],char,char,int);
int Scan_diag(char (*)[size],char,char,int);
int Scan_rowscols(char (*)[size],char,char,int);
int main()
{
//-------------------------
char xo[size][size]={'1','2','3','4','5','6','7','8','9'};
char Fig_PC='X';
char Fig_Player='O';
int PC_move=0;
//-------------------------
printf("Welcom to Tic Tac Toe \n");
Print_board(xo);
Clear_board(xo);
//PC plays first and increments PC_move
PC_turn(xo,Fig_PC,Fig_Player,&PC_move);
//While nobody won, or PC reaches max move game is on
while ((Check_board(xo,Fig_PC,&PC_move)))
{
Player_turn(xo,Fig_Player);
PC_turn(xo,Fig_PC,Fig_Player,&PC_move);
}
return 0;
}
void PC_turn(char (xo)[size],char Fig_PC,char Fig_Player,int PC_move)
{
printf("Computer's Turn:\n");
/*
First try to win by completing an X X X
Second priority is to defend
Lastly try to take postion of corners
*/
if (!Action(xo,Fig_PC,Fig_Player,1))
{
if (!Action(xo,Fig_Player,Fig_PC,0))
{
Scan_corners(xo,Fig_PC,Fig_Player);
}
}
Print_board(xo);
++*PC_move;
}
void Print_board(char (*xo)[size])
{
int i=0;
printf("\n\n | | \n");
printf(" %c | %c | %c\n", xo[2][0], xo[2][1], xo[2][2]);
printf("___|___|___ \n");
printf(" | | \n");
printf(" %c | %c | %c\n", xo[1][0], xo[1][1], xo[1][2]);
printf("___|___|___ \n");
printf(" | | \n");
printf(" %c | %c | %c\n", xo[0][0], xo[0][1], xo[0][2]);
printf(" | | \n\n\n");
usleep(1000000);
}
void Clear_board(char (*xo)[size])
{
int i=0;
int j=0;
for (i=0;i
Solution
Here are some things that may help you improve your code.
Fix your formatting
The indentation, in particular, seems rather random. It may be that it's an artifact of posting the code, but it doesn't look very consistent. It's less important which coding convention you follow, than that you consistently follow some convention.
Omit
When a C or C++ program reaches the end of
Note: when I make this suggestion, it's almost invariably followed by one of two kinds of comments: "I didn't know that." or "That's bad advice!" My rationale is that it's safe and useful to rely on compiler behavior explicitly supported by the standard. For C, since C99; see ISO/IEC 9899:1999 section 5.1.2.2.3:
[...] a return from the initial call to the
For C++, since the first standard in 1998; see ISO/IEC 14882:1998 section 3.6.1:
If control reaches the end of main without encountering a return statement, the effect is that of executing return 0;
All versions of both standards since then (C99 and C++98) have maintained the same idea. We rely on automatically generated member functions in C++, and few people write explicit
So I advocate omitting it; others disagree, (often vehemently!) In any case, if you encounter code that omits it, you'll know that it's explicitly supported by the standard and you'll know what it means.
Eliminate unused variables
Unused variables are a sign of poor code quality, so eliminating them should be a priority. In this code,
Make sure you have all required
The code uses
Avoid obsolete functions
The
Think of the user
The square numbering is only shown once at the beginning of the game, so if the player doesn't remember that the squares are numbered from bottom to top (like the
Fix your formatting
The indentation, in particular, seems rather random. It may be that it's an artifact of posting the code, but it doesn't look very consistent. It's less important which coding convention you follow, than that you consistently follow some convention.
Omit
return 0When a C or C++ program reaches the end of
main the compiler will automatically generate code to return 0, so there is no need to put return 0; explicitly at the end of main. Note: when I make this suggestion, it's almost invariably followed by one of two kinds of comments: "I didn't know that." or "That's bad advice!" My rationale is that it's safe and useful to rely on compiler behavior explicitly supported by the standard. For C, since C99; see ISO/IEC 9899:1999 section 5.1.2.2.3:
[...] a return from the initial call to the
main function is equivalent to calling the exit function with the value returned by the main function as its argument; reaching the } that terminates the main function returns a value of 0.For C++, since the first standard in 1998; see ISO/IEC 14882:1998 section 3.6.1:
If control reaches the end of main without encountering a return statement, the effect is that of executing return 0;
All versions of both standards since then (C99 and C++98) have maintained the same idea. We rely on automatically generated member functions in C++, and few people write explicit
return; statements at the end of a void function. Reasons against omitting seem to boil down to "it looks weird". If, like me, you're curious about the rationale for the change to the C standard read this question. Also note that in the early 1990s this was considered "sloppy practice" because it was undefined behavior (although widely supported) at the time. So I advocate omitting it; others disagree, (often vehemently!) In any case, if you encounter code that omits it, you'll know that it's explicitly supported by the standard and you'll know what it means.
Eliminate unused variables
Unused variables are a sign of poor code quality, so eliminating them should be a priority. In this code,
i within Print_board is defined and assigned a value but then never used. Your compiler is probably also smart enough to tell you that, if you ask it to do so. Make sure you have all required
#includesThe code uses
usleep but doesn't #include . But with that said, see the next suggestion.Avoid obsolete functions
The
usleep call is a POSIX rather than a C standard function, but has been obsolete since POSIX.1-2001. You could either use nanosleep which is the new POSIX function or, since you are trying to delay 1 second and already have ` included, you could use sleep(1); instead.
Use whitespace to improve readability
Lines like this:
for (i=0;i<size;++i)
become much easier to read with a little bit of whitespace:
for (i = 0; i < size; ++i)
Use named constants
Instead of having "magic numbers" like 100 and 102 within the code, it would make more sense to make them named variables. This increases the readability and maintainability of the code and costs nothing in terms of performance.
Be consistent with initialization of variables
The xo variable is currently declared and initialized like this:
char xo[size][size] = { '1', '2', '3', '4', '5', '6', '7', '8', '9' };
but that's not quite right. It's declared as 3 sets of 3, so the initialization should instead be:
char xo[size][size] = { {'1', '2', '3'}, {'4', '5', '6'}, {'7', '8', '9' } };
Use const where practical
The Fig_PC and Fig_Player variables don't ever and shouldn't ever change and therefore should be declared const.
Declare variables in the smallest practical scope
By declaring variables in the smallest practical scope, you reduce the chance for name collisions and make it clear to the reader of your code where variables are and are not needed. With any C compiler conforming to the 1999 specification (which should be all of them at this point!) one could rewrite Clear_board like this for example:
void Clear_board(char (*xo)[size])
{
for (int i = 0; i < size; ++i) {
for (int j = 0; j < size; ++j) {
xo[i][j] = ' ';
}
}
}
The variables i and j are declared within the scope of their respective for loops. Although as a practical matter, I'd write it differently; see the next suggestion.
Use library functions where appropriate
The Clear_board function sets all of the members of xo to the ' '` character, but there's already a library function that does that very efficiently. Use it like this:void Clear_board(char (*xo)[size])
{
memset(xo, ' ', size*size);
}Think of the user
The square numbering is only shown once at the beginning of the game, so if the player doesn't remember that the squares are numbered from bottom to top (like the
Code Snippets
for (i=0;i<size;++i)for (i = 0; i < size; ++i)char xo[size][size] = { '1', '2', '3', '4', '5', '6', '7', '8', '9' };char xo[size][size] = { {'1', '2', '3'}, {'4', '5', '6'}, {'7', '8', '9' } };void Clear_board(char (*xo)[size])
{
for (int i = 0; i < size; ++i) {
for (int j = 0; j < size; ++j) {
xo[i][j] = ' ';
}
}
}Context
StackExchange Code Review Q#131629, answer score: 26
Revisions (0)
No revisions yet.