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

Tic Tac Toe vs PC

Submitted by: @import:stackexchange-codereview··
0
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

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 return 0

When 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 #includes

The 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.