I have seen this a while back, but it was late and I decided to reply the next morning, which I forgot.
But better late than never, here are 2 suggestions that I would make:
1. On main.cpp where you find :
if (player == '1'){
cout << player1 << " - it's your turn to play!\n";
}
else {
cout << player2 << " - it's your turn to play!\n";
}
if (player == '1') {
cout << "You play the X!\n";
XorO = 'X';
}
else {
cout << "You play the O!\n";
XorO = 'O';
}
It should be this:
if (player == '1'){
cout << player1 << " - it's your turn to play!\n";
cout << "You play the X!\n";
XorO = 'X';
}
else {
cout << player2 << " - it's your turn to play!\n";
cout << "You play the O!\n";
XorO = 'O';
}
And why is that?
You basically have something like this:
condition "A":
Action "B"
else:
Action "C"
condition "A":
Action "D"
else:
Action "E"
So unless the action "B" or "C" changes the condition "A" (or A does a tangent operation that does something else, which isn't the case), then if "A" is true after action B is performed then action D will always follow as well. Similarly if "A" is false it will execute C first then it will always execute "E".
So if you write it like this:
condition "A":
Action "B"
Action "D"
else:
Action "C"
Action "E"
It will perform the actions just the same, except that now you don't need to repeat the operation of checking the A.
2. On hearder.h similarly, where it reads:
void Board::show(void){
for (int line = 0; line<3; line++) {
if (line == 0){
cout << Line << endl;
}
for (int b = 0; b<2; b++){ // Each X/O has two lines for /'s and \'s.
for (int column = 0; column<3; column++) { switch (spot[line][column]) {
case 'X':
if (b == 0)
cout << Cross1; // First line for the X.
else
cout << Cross2; // Second line for the X.
break;
case 'O':
if (b == 0)
cout << Circle1;
else
cout << Circle2;
break;
default:
cout << Empty;
break;
}
}
cout << Column << endl;
}
cout << Line << endl;
}
}
It should be:
void Board::show(void){
[color=red]cout << Line << endl;[/color]
for (int line = 0; line<3; line++) {
for (int b = 0; b<2; b++){
for (int column = 0; column<3; column++) { switch (spot[line][column]) {
case 'X':
if (b == 0)
cout << Cross1; // First line for the X.
else
cout << Cross2; // Second line for the X.
break;
case 'O':
if (b == 0)
cout << Circle1;
else
cout << Circle2;
break;
default:
cout << Empty;
break;
}
}
cout << Column << endl;
}
cout << Line << endl;
}
}
Because printing the first line will always happen before everything else, and more than that it will only happen once, so you don't need to force a check on this parameter for every "for" cycle.
I don't know if this helps, but you can think in terms of states, dependencies and the minimal amount of operations that you need to perform.
If one operation is independent of the one being executed before, then you can switch the order of the 2. If your operation need only to be performed once, only once and always once then you don't need to check if you want to perform that operation or not, you just do it once and don't put it in a cycle. You may need to check for a certain condition A to select either to perform certain operations or not, if you code smartly I would say that 90% of the cases you only have to check for that condition once in your entire code, if you do it more than once while your condition is not expected to change then you are probably doing something inefficient. Forget the temporal order by witch you would mentally perform the operations, do it instead in the order that will minimize the number of steps required to complete the task, and that may mean changing the order of things around (as long as they are interchangeable ofcourse). But I fear that sensitivity for this sort of things may only come from practice.
Despite that, it is a good progress.
"I have an irrefutable argument for the existence of...." NO, STOP! You are already wrong!