Tic Tac Toe

[goth]

Hello. I've been playing around with C++ for a while but only lately(the last month) I've really got into it. This is my first (real) program. I know it's a mess but I plan on separating some of the larger functions into different .cpp files and also make a header.

What I want is some feedback and what I've done wrong/what I can improve.

Thanks.

#include <iostream>
using namespace std;

bool GameOver;
int CheckWinners (char board[3][3],int player);
int DrawTable (char board[3][3]);

int main()
{
	bool ValidMove;
	int PlayerChoice;
	int PlayerTurn = 1;
	char PlayerMark = 'x';
	char board[3][3]=
	{
		//0   1   2
		{'1','2','3'},	//0
		{'4','5','6'},	//1
		{'7','8','9'}	//2
	};

	
	cout << "\t\t\tWelcome to Tic Tac Toe!\n\n\n" << endl;			//welcome screen

	do{																//main game loop

		do{

			DrawTable(board);

			if (PlayerTurn == 1)
			{
				PlayerMark = 'x';
			}

			else if (PlayerTurn == 2)
			{
				PlayerMark = 'o';
			}

			cout << "\nIt's player " << PlayerTurn << "'s turn: ";

			cin >> PlayerChoice;
		
			ValidMove = 1;

			if (PlayerChoice == 1 && board[0][0] == '1')            
				{
					board[0][0] = PlayerMark;
				}

			else if (PlayerChoice == 2 && board[0][1] == '2')
				{
					board[0][1] = PlayerMark;
				}
		
			else if (PlayerChoice == 3 && board[0][2] == '3')
				{
					board[0][2] = PlayerMark;
				}

			else if (PlayerChoice == 4 && board[1][0] == '4')
				{
					board[1][0] = PlayerMark;
				}

			else if (PlayerChoice == 5 && board[1][1] == '5')
				{
					board[1][1] = PlayerMark;
				}

			else if (PlayerChoice == 6 && board[1][2] == '6')
				{
					board[1][2] = PlayerMark;
				}

			else if (PlayerChoice == 7 && board[2][0] == '7')
				{
					board[2][0] = PlayerMark;
				}

			else if (PlayerChoice == 8 && board[2][1] == '8')
				{
					board[2][1] = PlayerMark;
				}

			else if (PlayerChoice == 9 && board[2][2] == '9')
				{
					board[2][2] = PlayerMark;
				}

			else
				{
					cout << "\nInvalid Move!\n";
					ValidMove = 0;
					cin.ignore();cin.get();
				}

		}while (!ValidMove);

		CheckWinners(board,PlayerTurn);

	if (PlayerTurn == 1)
		{
			PlayerTurn = 2;
		}
	else 
		{
			PlayerTurn = 1;
		}

	} while (!GameOver);

cout << "\nThanks for playing!";
	
cin.ignore();cin.get();
	
}

int DrawTable (char board[3][3])
{
	for (int i = 0; i<3; i++)								
	{
			cout << " - - -" << "\n";
			
			for (int j = 0; j<3; j++)
				{
					cout << "|" << board[i][j];
				}
	
			cout << "|" << "\n" ;
				
	}

return 0;
}

int CheckWinners(char board[3][3], int player)
{
	if (board[0][0] == board [0][1] && board [0][2] == board[0][1])			//upper line
		{
			DrawTable(board);
			cout << "\nPlayer " << player << " won!\n";
			GameOver = 1;
			return 1;
			cin.ignore();cin.get();
		}

	else if (board[2][2] == board [2][1] && board [2][0] == board[2][1])	//bottom line
		{
			DrawTable(board);
			cout << "\nPlayer " << player << " won!\n";
			GameOver = 1;
			return 1;
			cin.ignore();cin.get();
		}

	else if (board[0][0] == board [1][0] && board [2][0] == board[1][0])	//left line
		{
			DrawTable(board);
			cout << "\nPlayer " << player << " won!\n";
			GameOver = 1;
			return 1;
			cin.ignore();cin.get();
		}

	else if (board[2][2] == board [1][2] && board [0][2] == board[1][2])	//right line
		{
			DrawTable(board);
			cout << "\nPlayer " << player << " won!\n";
			GameOver = 1;
			return 1;
			cin.ignore();cin.get();
		}

	else if (board[0][0] == board [1][1] && board [2][2] == board[1][1])	//diagonal 1
		{
			DrawTable(board);
			cout << "\nPlayer " << player << " won!\n";
			GameOver = 1;
			return 1;
			cin.ignore();cin.get();
		}

	else if (board[0][2] == board [1][1] && board [2][0] == board[1][1])	//diagonal 2
		{
			DrawTable(board);
			cout << "\nPlayer " << player << " won!\n";
			GameOver = 1;
			return 1;
			cin.ignore();cin.get();
		}

	else if (board[1][0] == board [1][1] && board [1][2] == board[1][1])	//center horizontal
		{
			DrawTable(board);
			cout << "\nPlayer " << player << " won!\n";
			GameOver = 1;
			return 1;
			cin.ignore();cin.get();
		}

	else if (board[0][1] == board [1][1] && board [2][1] == board[1][1])	//center vertical
		{
			DrawTable(board);
			cout << "\nPlayer " << player << " won!\n";
			GameOver = 1;
			return 1;
			cin.ignore();cin.get();
		}

	else if (board[0][0] != '1' && board[0][1] != '2' && board[0][2] != '3' && board[1][0] != '4' && board[1][1] != '5' && board[1][2] != '6' && board[2][0] != '7' && board[2][1] != '8' && board[2][2] != '9')
		{
			DrawTable(board);
			cout << "\nDraw!";
			GameOver = 1;
			return 1;
			cin.ignore();cin.get();
		}
	return 0;
}

/*
 * Copyright (c) 2011, Puscasu Emanuel (GothicFighter)
 */

[eraggo]

You don't need if-statement in line 123. Move cout inside there before for loop; see?

Also check some intends from original code:
-Each for loops, if-statements, etc
Compare this (lines 215-222)

else if (board[0][0] != '1' && board[0][1] != '2' && board[0][2] != '3' && board[1][0] != '4' && board[1][1] != '5' && board[1][2] != '6' && board[2][0] != '7' && board[2][1] != '8' && board[2][2] != '9')
	{
		DrawTable(board);
		cout << "\nDraw!";
		GameOver = 1;
		return 1;
		cin.ignore();cin.get();
	}

and this:

else if (board[0][0] != '1' && board[0][1] != '2' && board[0][2] != '3' && board[1][0] != '4' && board[1][1] != '5' && board[1][2] != '6' && board[2][0] != '7' && board[2][1] != '8' && board[2][2] != '9')
{
	DrawTable(board);
	cout << "\nDraw!";
	GameOver = 1;
	return 1;
	cin.ignore();cin.get();
}

AND even more this:

else if (board[0][0] != '1' && board[0][1] != '2' && board[0][2] != '3' && board[1][0] != '4' && board[1][1] != '5' && board[1][2] != '6' && board[2][0] != '7' && board[2][1] != '8' && board[2][2] != '9') {
	DrawTable(board);
	cout << "\nDraw!";
	GameOver = 1;
	return 1;
	cin.ignore();cin.get();
}

[goth]

Thanks for the answer. I still can't figure my way on how to intend the code to be as readable as possible. The second option seems better.

[eraggo]

Yes, second WOULD be excellent choice (for Window$ users). But for Linux users where most lines of code is changed in... well.. terminal.

Even that computers today have high resolutions, Linux consoles by default (in ubuntu atleast) have 80*24 lines which makes reading second option is a pain. And of course Linux users are lazy.. That's why you should learn third option i gived to yoy (even you are not currently programming to Linux developer you should try. You never know how long Window$ is in business)

Have a nice time in coding :P

[Intrexa]

Lines 46-86 can be consolidated

if (PlayerChoice > 0 && PlayerChoice < 10 && board[(PlayerChoice-1)/3][(PlayerChoice-1)%3] == char(PlayerChoice + '0')            
				{
					board[PlayerChoice/3][PlayerChoice%3] = PlayerMark;
				}
 else{
					cout << "\nInvalid Move!\n";
					ValidMove = 0;
					cin.ignore();cin.get();
				}

PlayerChoice > 0 && PlayerChoice < 10 Is validating the input, so that later you don't run out of bounds. C++ evaluates conditions from left to right, and as soon as it finds one that is false, it stops evaluating, so the validating is done before you try to access an array element that is out of bounds.

(PlayerChoice-1)/3 will evaluate to the size column properly, so if the user inputs 3, minus 1 is 2, divided by 3, integer division always rounds down to 0. If the user inputs 4, minus 1 is 3, divided by 3 is one, so it would be in column 1 (the second column).

(PlayerChoice-1)%3 likewise does the previous, then chooses the row based on the remainder.

char(PlayerChoice + '0') converts a single digit number to the character equivalant. It adds the decimal value of '0' (the first number in ascii, 48) to the number the user entered, then converts that to a char. So if the user enters 3, 3 + 48 is 51, 51 as a char is '3', which would then be compared to the char held in the array.

[goth]

Math is not my virtue (aka I totally suck) so thanks for explaining the code but I've tried it and it doesn't work correctly. If I type "1", square number "2" gets completed and so on.

[Intrexa]

Oops, my bad, my line 3 was wrong, should be board[(PlayerChoice-1)/3][(PlayerChoice-1)%3]

[goth]

That works allright :). Now, I've fixed some major problems(specifically the recognition pattern for diagonals was screwed up) and I encounter a minor bug. When I enter a letter instead of a number the program gets into an infinite loop (it keeps drawing the table and says "Invalid move".

Any ideas on how to fix this?

[eraggo]

BTW: why board has 3x3 chars instead using ints?

declare each values to be 0 to denote current "cell" is free... (remember:

if (cell has something)
  there is no point to check if this tile is occupied by current or another player

)

[goth]

I thought the program would be less resource-intensive if I used chars instead of ints.

Also, your idea is great but I want the numbers to be there as a kind of visual aid.

[eraggo]

agreed for that (some sorts of it)...
using tables (3x3) you would have benefit to check if CURRENT player has won... There is possibility there to expand your program to handle "5 on row" if you follow my example :P

[masky007]

you can use a for loop to check for winning combinations.. first for the rows and another one for the columns (and two separate for the diagonals) this could shorten your code.
As well you could use for loop to generate the table.
here is a link to my tic tac toe i have done recently:
http://www.cplusplus.com/forum/beginner/49729/
you could get something out of it (btw i use multidimensional vector instead array)