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

4×4 Tic-Tac-Toe in C

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

Problem

While trying to teach myself C, I decided to take a stab at writing this Tic-Tac-Toe program on a 4x4 board. It will be great if somebody could review it and let me know of some ways of refining this.

```
#include
#include

typedef struct {
int xpos;
int ypos;
} BRDPOS;

void printBoard(int brd_size,char board[][brd_size]){
int i,j;
for(i = 0;i < brd_size;i++){
for(j = 0;j < brd_size;j++){
if(j == 0) printf("| ");
printf("%c |",(char)board[i][j]);
if(j == (brd_size - 1)){
printf("\n");
}
}
}
}

BRDPOS getPosition(int brd_size,char board[][brd_size]){
//Enter Positions X and Y
BRDPOS pos;
int x,y;
do{
printf("Enter the x-position:");
scanf("%d",&x);
printf("\nEnter the y-position:");
scanf("%d",&y);
printf("\n");
}while(board[x][y] != ' ');
pos.xpos = x;
pos.ypos = y;
return pos;
}

void updateBoard(int brd_size,char board[][brd_size],BRDPOS pos,int player){
if(player == 0){
board[pos.xpos][pos.ypos] = 'X';
}else{
board[pos.xpos][pos.ypos] = 'O';
}
}

int isWinningPosition(BRDPOS position,int brd_size,char brd[][brd_size],int player){
char posChar = (player == 0)? 'X':'Y';
int xPos = position.xpos;
int yPos = position.ypos;
char yStatus = brd[0][0];
char majDiag = brd[0][0];
char minDiag = brd[0][brd_size-1];
int xstatus = 0;
int ystatus = 0;
int majdiag = 0;
int mindiag = 0;
int i,j,k,l;
for(i=0;i< brd_size;i++){
if(brd[xPos][yPos] == brd[xPos][i]){
xstatus = 1;
}else{
xstatus = 0;
break;
}
}
//Check if the horizontal has the same characters
for(j=0;j<brd_size;j++){
if(brd[xPos][yPos] == brd[j][yPos]){
ystatus = 1;
}else{
ystatus = 0;
break;
}
}
//Check if the maj

Solution

General:

I myself am writing a Tic-Tac-Toe game of sorts, so I am go to try to not give my code away too much. For a self-taught beginning C programmer, this code looks good. However, it looks like you were over thinking things a bit based on how long your code is.

What you did well on:

-
You kept your dependencies at a minimum (for the most part).

-
You attempt to use comments to describe what your thought process is and how the program works.

-
You used typedef with your struct properly.

-
You used a switch instead of multiple if-else test conditions (very good!).

Things you could improve on:

-
You unnecessarily #include .

-
You aren't using the characters that correspond with a Tic-Tac-Toe game.

char posChar = (player == 0)? 'X':'Y';


I don't see any reason to not use the X's and O's. We want to be as user-experience friendly as possible to lowering the initial learning curve for first time users.

char posChar = (player == 0) ? 'X' : 'O';


-
Your name of the typedef struct is not how the usual programmer would name it.

typedef struct {
       int xpos;
       int ypos;
   } BRDPOS;


Right now you are using the naming standards that coincide with naming a macro. Here is how I would name a similar struct.

typedef struct
{
    int xPos;
    int yPos;
} BoardPosition;


-
You have some unused variables: posChar, yStatus, and player. Get rid of them.

-
I would #define your board size at the top.

#define ROWS 4
#define COLS 4


-
You always set some variables in a function to your struct values.

int xPos = position.xpos;
 int yPos = position.ypos;


I'm not seeing the purpose of using these local variables at all. I would remove them and use the direct struct values in their place.

-
Initialize variables in your for loop, not outside of them.(C99)

for (int row = 0; row < ROWS; row++)


-
Your comments are sporadic. Also, some areas have too many comments.

//Ask position to place the X for player 1
   //getPosition keeps asking for the position until a legal position has been entered
   printf("Enter positions for PlayerA:\n");
   BRDPOS positionA = getPosition(4,board);


Here is an example of where I think you are saying too much with your comments. Some code can speak for itself. However, in other areas you have no comments at all.

return xstatus|ystatus|majdiag|mindiag;


Some people may not know what the bitwise operator | does. You should comment more in areas like these that may be more confusing to newer developers.

-
You could use the function puts() in place of your printf().

printf("Enter position for PlayerB:\n");


Using puts() is a bit better in my mind. Then you don't have to worry about the \n character at the end.

puts("Enter position for PlayerB: ");

Code Snippets

char posChar = (player == 0)? 'X':'Y';
char posChar = (player == 0) ? 'X' : 'O';
typedef struct {
       int xpos;
       int ypos;
   } BRDPOS;
typedef struct
{
    int xPos;
    int yPos;
} BoardPosition;
#define ROWS 4
#define COLS 4

Context

StackExchange Code Review Q#41304, answer score: 15

Revisions (0)

No revisions yet.