-
Code improving
He guys,
I made a Tictactoe game and since I am learning C++ myself, can you tell me how to improve my coding?
The main.cpp file isn't important, it only contains the menu.
Code:
#include <iostream>
#include <cstdlib>
#include <windows.h>
using namespace std;
void pause();
void newgame();
void x(char board[10]);
void y(char board[10]);
void drawboard(char board[10]);
void play();
int win(char board[10]);
int win(char board[10])
{
if(board[1] == 'X' && board[2] == 'X' && board[3] == 'X')
{
return 1;
}
else if(board[1] == 'O' && board[2] == 'O' && board[3] == 'O')
{
return 1;
}
else if(board[4] == 'X' && board[5] == 'X' && board[6] == 'X')
{
return 1;
}
else if(board[4] == 'O' && board[5] == 'O' && board[6] == 'O')
{
return 1;
}
else if(board[7] == 'X' && board[8] == 'X' && board[9] == 'X')
{
return 1;
}
else if(board[7] == 'O' && board[8] == 'O' && board[9] == 'O')
{
return 1;
}
else if(board[3] == 'X' && board[6] == 'X' && board[9] == 'X')
{
return 1;
}
else if(board[3] == 'O' && board[6] == 'O' && board[9] == 'O')
{
return 1;
}
else if(board[2] == 'X' && board[5] == 'X' && board[8] == 'X')
{
return 1;
}
else if(board[2] == 'O' && board[5] == 'O' && board[8] == 'O')
{
return 1;
}
else if(board[1] == 'X' && board[4] == 'X' && board[7] == 'X')
{
return 1;
}
else if(board[1] == 'O' && board[4] == 'O' && board[7] == 'O')
{
return 1;
}
else if(board[1] == 'X' && board[5] == 'X' && board[9] == 'X')
{
return 1;
}
else if(board[1] == 'O' && board[5] == 'O' && board[9] == 'O')
{
return 1;
}
else if(board[3] == 'X' && board[5] == 'X' && board[7] == 'X')
{
return 1;
}
else if(board[3] == 'O' && board[5] == 'O' && board[7] == 'O')
{
return 1;
}
else if(board[1] != ' ' && board[2] != ' ' && board[3] != ' ' && board[4] != ' ' && board[5] != ' ' && board[6] != ' '
&& board[7] != ' ' && board[8] != ' ' && board[9] != ' ')
{
return -1;
}
else
{
return 0;
}
}
void x(char board[10])
{
bool full = true;
int number;
int won;
bool cont = false;
cout << "X turn, enter number (1-9): ";
while(cont == false)
{
while(!(cin >> number))
{
cin.clear();
cin.ignore(1000, '\n');
cout << "Invalid input! Please re-enter: ";
}
if(number > 9 || number < 1)
{
cout << "Number can't be smaller than 1 or larger than 9! ";
cont = false;
}
else if(board[number] == ' ')
{
board[number] = 'X';
cont = true;
}
else
{
cout << "Not possible, try again! ";
cont = false;
}
}
system("cls");
drawboard(board);
won = win(board);
if(won == 1)
{
cout << "X won!\n";
return;
}
else if(won == -1)
{
cout << "Tie!\n";
return;
}
else
{
y(board);
}
}
void y(char board[10])
{
int number;
int won;
bool cont = false;
cout << "O turn, enter number (1-9): ";
while(cont == false)
{
while(!(cin >> number))
{
cin.clear();
cin.ignore(1000, '\n');
cout << "Invalid input! Please re-enter: ";
}
if(number > 9 || number < 1)
{
cout << "Number can't be smaller than 1 or larger than 9! ";
cont = false;
}
else if(board[number] == ' ')
{
board[number] = 'O';
cont = true;
}
else
{
cout << "Not possible, try again! ";
cont = false;
}
}
system("cls");
drawboard(board);
won = win(board);
if(won == 1)
{
cout << "O won!\n";
return;
}
else if(won == -1)
{
cout << "Tie!\n";
return;
}
else
{
x(board);
}
}
void drawboard(char board[10])
{
cout << board [1] << " | " << board[2] << " | " << board[3] << endl;
cout << "---------" << endl;
cout << board [4] << " | " << board[5] << " | " << board[6] << endl;
cout << "---------" << endl;
cout << board [7] << " | " << board[8] << " | " << board[9] << endl;
}
void newgame()
{
system("cls");
char board[10];
for(int i =1; i <=10; i++)
{
board[i] = ' ';
}
drawboard(board);
x(board);
}
void pause()
{
cin.clear();
cin.ignore(1000, '\n');
cout << "Press enter to continue.";
cin.get();
}
-
Try using some for() loops; it will shorten your program considerably, and make updating and maintaining the code easier. But your code is not bad. :)
PS. Making it object oriented is also a good idea ;)
-
You should get used to arrays starting at index 0 instead of 1 :)
Your x and y functions seems to do the same thing. You should prolly use one function with some argument telling which player's turn it is.
Also, it seems game progression is done recursively. In this case it may not matter (as the depth will not exceed 9) but in general you'd don't want this recursive structure.
-
thanks for the replies! That's what I need! :D
About the array starting at 1, I thought it would be easier:
1 | 2 | 3
----------
4 | 5 | 6
----------
7 | 8 | 9
-
How about just
0 | 1 | 2
----------
3 | 4 | 5
----------
6 | 7 | 8
I believe <= is slower than or equal to <, anyway.
Besides, everyone else counts from 0, and everyone's code will do so too. You might as well live with it.
-
I did that once before, count from 1 instead of 0. Make it easier to read. Well, everyone has a choice. The array has an extra element.
-
You could abstract the cell index from the physical index using a converter, like CellToIndex:
Code:
int CellToIndex(int Cell)
{
return Cell - 1;
}
The cell would be 1-9 but the index 0-8.
Or perhaps use enumerations:
Code:
enum CELL
{
CELL_1,
CELL_2,
CELL_3,
CELL_4,
CELL_5,
CELL_6,
CELL_7,
CELL_8,
CELL_9,
}
(giving them values 0-8)