Avoid platform specific code
If there is an alternative you should try to avoid platform specific code.
This entails avoiding #include <windows.h> and using things like system("pause"); which rely on commands the OS understands.
There are guides on how to wait for user input on the command line and on how to clear the screen on multiple platforms on SO.
I had to adapt your code to make it run on my machine.
Use arrays for repetitive data
You should definitely use an array instead of single variables.
char x1,x2,x3,x4,x5,x6,x7,x8,x9;
would become
char fields[9];
or even two dimensional
char fields[3][3];
You would have to change to zero based indexing for this so x4 would become fields[3] or fields[1][0] in the two dimensional case.
Use loops for repetitive tasks
The array opens up loop usage. For example, you might want to do the printing in a loop instead of a row. For the 2D array this would be:
for(int row = numberOfRows - 1; row >= 0; --row) {
for(int col = 0; col < numberOfColumns; ++col) {
cout << fields[row][col];
if(col < 2) {
cout << "I";
} else {
cout << "\n";
}
}
}
There are multiple occasions where you print the board in some way or another.
This calls for a function that does the printing:
void print(Board fields, std::string columnSeparator = "I", std::string rowSeparator = "\n") {
for(int row = 2; row >= 0; --row) {
for(int col = 0; col < 3; ++col) {
cout << fields[row][col];
if(col < 2) {
cout << columnSeparator;
} else {
cout << rowSeparator;
}
}
}
}
This function can be used like:
print(fields);
cout << "Tic Tac Toe\n";
or like
Board numbers{{'1', '2', '3'},
{'4', '5', '6'},
{'7', '8', '9'}};
print(numbers);
Don't abuse the comma operator
In your code you have many lines like
if(x==1 && x1!='X' && x1!='O')if(a%2==0)x1='X', a++, i++;
else x1='O', a++, i++;
You should not misuse the comma operator to chain multiple commands into an if. Instead, you should separate the commands by ; as usual and group them into a common scope like so:
if(x==1 && x1!='X' && x1!='O') {
if(a%2==0) {
x1='X';
a++;
i++;
} else {
x1='O';
a++;
i++;
}
}
Factor out common code
There are many similarities in the different branches of the if in the code above. When the trailing code of two branches of the same if is the same, you can extract it out of the if:
if(x==1 && x1!='X' && x1!='O') {
if(a%2==0) {
x1='X';
} else {
x1='O';
}
a++;
i++;
}
Generally, you should always use {} after control flow constructs like if, while, or for to avoid errors where adding a line does not add it to the correct loop/if.
Use the ternary operator where it improves readability
The nested if in the above code can be replaced by the ternary operator:
if(a%2==0) {
x1='X';
} else {
x1='O';
}
becomes
x1= (a%2==0) ? 'X' : 'O';
Separate validity checks from actions
Finally, you have the list of ifs that should set the appropriate field.
First, we need to adapt that to the 2D array. To do so, we have to calculate the row and column from the position x:
const int row = (x - 1) / numberOfColumns;
const int column = (x - 1) % numberOfColumns;
if(fields[row][column] == '+') {
fields[row][column] = (a%2==0) ? 'X' : 'O';
a++;
i++;
}
Duplicate variables
Reducing the code showed that variables i and a do exactly the same and will therefore have the same value. So why have two of them?
I also noticed that a was not initialized which results in undefined behavior.
Check user input, return meaningful errors
Every time we read values from the user, we have to consider what happens when the user enters an invalid value.
While your code does nothing wrong thanks to the many ifs, it would be better to make the input checks more explicit in the code and to tell the user what has gone wrong:
do{
cout << "Type coordinate of square(number) ";
cin >> x;
if(x < 1 || x > 9) {
cout << "Invalid value: Expected integer between 1 and 9!\n";
}
} while(!(0 < x && x < 10));
and similarly:
if(fields[row][column] == '+') {
fields[row][column] = (i%2==0) ? 'X' : 'O';
i++;
} else {
cout << "Field already taken!\n";
}
Naming
Names are important for the understanding of variables' meanings.
Generally, the farther away from the declaration a name is used the more descriptive it needs to be.
In your code I would rename as follows:
x1, x2, ... -> fieldsx -> fieldIndexi -> roundIndexa -> removed as it duplicates i
Misc
You don't need to return 0; from main in C++. This is done by default if main ends before a return is encountered.
Don't using namespace std; it will bring you more trouble than it saves in typing.
Use correct formatting to improve the readability of your code. Usually, each level of nesting ({}) deepens the indentation.
Code
This is the code that I came up with. There are still some issues that I would correct but this post has become long enough as it is. (Note that I have removed the screen clearing but it should be possible to reinsert it in a multi platform manner.)
#include <iostream>
constexpr int numberOfRows = 3;
constexpr int numberOfColumns = 3;
using Board = char[numberOfRows][numberOfColumns];
void print(Board fields, std::string columnSeparator = "I",
std::string rowSeparator = "\n") {
for (int row = numberOfRows - 1; row >= 0; --row) {
for (int col = 0; col < numberOfColumns; ++col) {
std::cout << fields[row][col];
if (col < numberOfColumns - 1) {
std::cout << columnSeparator;
} else {
std::cout << rowSeparator;
}
}
}
}
int main() {
Board fields;
for (int row = 0; row < numberOfRows; ++row) {
for (int column = 0; column < numberOfColumns; ++column) {
fields[row][column] = '+';
}
}
int fieldIndex, roundIndex = 0;
print(fields);
std::cout << "Tic Tac Toe\n";
std::cin.get();
Board numbers{{'1', '2', '3'}, {'4', '5', '6'}, {'7', '8', '9'}};
print(numbers);
std::cout << "Board with coordinates\n";
std::cin.get();
while (roundIndex < 9) {
print(fields);
bool xsTurn = roundIndex % 2 == 0;
if (xsTurn) {
std::cout << "X Turn\n";
} else {
std::cout << "O Turn\n";
}
do {
std::cout << "Type coordinate of square(number) ";
std::cin >> fieldIndex;
if (fieldIndex < 1 || fieldIndex > 9) {
std::cout << "Invalid value: Expected integer between 1 and 9!\n";
}
} while (!(0 < fieldIndex && fieldIndex < 10));
const int row = (fieldIndex - 1) / numberOfColumns;
const int column = (fieldIndex - 1) % numberOfColumns;
if (fields[row][column] == '+') {
fields[row][column] = (xsTurn) ? 'X' : 'O';
roundIndex++;
} else {
std::cout << "Field already taken!\n";
}
}
print(fields);
std::cout << "Who won?\n";
std::cin.get();
}
