Thread: Opinions?

can someone give me an opinion on how to improve my code? im trying to teach myself c++. btw, its tic tac toe

[code]
#include <iostream.h>
#include <stdlib.h>

char board[3][3];
void clearboard();
void showboard();
void getx();
void gety();
int checkboardx();
int checkboardy();



int main()
{
int loop=0;
clearboard();
showboard();

A:
getx();
loop++;
showboard();
if(checkboardx()==1)
{
cout<<"

*********
*X WINS!*
*********
\n";
system("PAUSE");
return 0;
}
if(loop==9)
{
cout<<"

*********
* TIE *
*********
\n";
system("PAUSE");
return 0;
}

gety();
loop++;
showboard();
if(checkboardy()==1)
{
cout<<"

*********
*O WINS!*
*********
\n";
system("PAUSE");
return 0;
}

goto A;

}

void clearboard()
{
int x,y;
for(x=0; x<3; x++)
{
for(y=0; y<3;y++)
{
board[x][y]='.';
}
}
}

void showboard()
{
int x,y,z;
y=0;
cout<<endl<<endl<<"Board Status: "<<endl;

cout<<"1 2 3"<<endl;
cout<<"-------"<<endl;


for(x=0; x<3; x++)
{
cout<<board[x][y]<<" ";
}
cout<<"| 1"<<endl;
y=1;
for(x=0; x<3; x++)
{
cout<<board[x][y]<<" ";
}
y=2;
cout<<"| 2"<<endl;
for(x=0; x<3; x++)
{
cout<<board[x][y]<<" ";
}

cout<<"| 3"<<endl<<endl<<endl;
}

void getx()
{
int x,y;
cout<<"
Place an X
-----------
Type the X coordinate: ";
cin>>x;
x=x-1;
cout<<"
Type the Y coordinate: ";
cin>>y;
y=y-1;

if(board[x][y]=='.')
{
board[x][y]='X';
}
else
{
cout<<"Already taken!\n";
getx();
}
system("CLS");
}

void gety()
{
int x,y;
cout<<"
Place a Y
---------
Type the X coordinate: ";
cin>>x;
x=x-1;
cout<<"
Type the Y coordinate: ";
cin>>y;
y=y-1;

if(board[x][y]=='.')
{
board[x][y]='O';
}
else
{
cout<<"Already taken!\n";
gety();
}
system("CLS");
}

int checkboardx()
{

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

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

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

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

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

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

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

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

int checkboardy()
{

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

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

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

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

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

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

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

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

I dunno, but it seems you're using basic c++, re-write the prog in more advanced c++ like classes templates etc etc...

The first step is to use indentation.

whenever a { is encountered, indent after it with TAB. When you are ready to put the closing }, go back to the previous indentation then place the brace.

=)

I suggest you improve your checkboard functions.

What I would do is instead of having two checkboard functions, just have one function, with a char as a parameter, where the parameter is the letter you're checking for. Now, that sounds very confusing, so here's an example:

Code:

int checkboard(char *letter)
{
 if (board[0][0]=letter && board[0][1]==letter && board[0][2]==letter)
 {
    return 1;
 }
 //etc.
}

Then, in main when you call your checkboard function, you just use checkboard('X') or checkboard('O'). That way you don't have to write out two checkboard functions, and your code is much more flexible.

Another thing you can do in your checkboard function is instead of having tons of if statements, use a loop that goes through each element in your board array and checks to see if the letter in that spot is in line with letters in any other spots. For a simple tic-tac-toe game, writing something like that would probably actually be harder than what you did, but if you were to decide you wanted a board with 50x50 squares in it, it would take forever to write that many if statements, whereas a loop would be much easier.

When you're checking the values of an array, it is generally good form in almost all cases to use a loop instead of a series of if statements. This allows for greater code flexibility and is often faster to code.