Trying to avoid bad habbits early.

[quwackers]

So this is my code for a game of tic tac toe, obviously it is not complete, I need to add in a restart component at the succession of a game and PC AI.

What my question is, is am I getting into any bad habbits, I want to get into good habbits now so as I learn and code more and more I do not do things incorrectly. If you could just go over my code quickly and let me know it'd be greatly appreciated. Cheers :)

#include <iostream>
#include <string>


using namespace std;


class TicTacToe {
	public:
		//This constructor sets all the values in the list to 0
		TicTacToe()
		{
			for (int i = 0; i < 10; i++)
				pos[i] = 0;
		}

		//Converts numbers (1 or 2) into X or O respectively from printing.
		string numToChar(int x) 
		{
			if(x == 1)
				return "X";
			else if(x == 2)
				return "O";
			else 
				return " ";
		}

		//Prints out the board and uses the numToChar() func to replace 1 and 2 with X or O.
		void printBoard()
		{
			cout << numToChar(pos[0]) << " | " << numToChar(pos[1]) << " | " << numToChar(pos[2]) << endl;
			cout << "----------"  << endl;
			cout << numToChar(pos[3]) << " | " << numToChar(pos[4]) << " | " << numToChar(pos[5]) << endl;
			cout << "----------"  << endl;
			cout << numToChar(pos[6]) << " | " << numToChar(pos[7]) << " | " << numToChar(pos[8]) << endl << endl;
		};

		void startUp(string multiOrSing)
		{

		if(multiOrSing == "multiplayer")
		{
				while(turns < 10)
				{
					humanMove(1);
					printBoard();
					winner();
					humanMove(2);
					printBoard();
					winner();
				}
		}
		else if(multiOrSing == "singleplayer")
		{
			cout << "How skilled would you like your opponent to be; begginer, easy, medium or hard? ";
			cin >> difficulty;
		}

		}
		
		//Accepts human input and sets it in the array pos[9] accordingly.
		void humanMove(int player) //Player is either 1 or 2
		{
			int move;

			cout << "Please enter your move Player " << player << ":  ";
			cin >> move;
			cout << endl;

			move--;

			//Makes sure no player has already marked the location and that the input value is between 1 and 9.
			if(pos[move] == 0 && move < 10 && move >= 0)
			{
				pos[move] = player;
				turns++;
			}
			else
			{
				cout << "INVALID MOVE." << endl << endl;
				humanMove(player);
			}
		}

		//Determins whether or not a player has won yet.
		void winner()
		{
			//This is a list of the winning combinations.
			int winCombo[8][3] = {{0, 1, 2}, {3, 4, 5}, {6, 7, 8}, {0, 3 , 6}, {1, 4, 7}, {2, 5, 8}, {0, 4, 8}, {2, 4, 6}};

			for(int x = 0; x < 8; x++)
			{
				if(pos[winCombo[x][0]] == pos[winCombo[x][1]] && pos[winCombo[x][1]] == pos[winCombo[x][2]] && pos[winCombo[x][0]] == 1)
					cout << "Player 1 wins!" << endl << endl;
				else if(pos[winCombo[x][0]] == pos[winCombo[x][1]] && pos[winCombo[x][1]] == pos[winCombo[x][2]] && pos[winCombo[x][0]] == 2)
					cout << "Player 2 wins!";
			}
		}

	private:
		int pos[9];
		int turns;
		string difficulty;
};


int main()
{

	TicTacToe ttt;

	string todo;

	cout << "Welcome to Tic Tac Toe!" << endl << endl << endl;

	cout << "Would you like to play a game of Tic Tac Toe? (yes/no) ";
	cin >> todo;
	cout << endl << endl << endl;

	if(todo == "yes")
	{		
	cout << "Below is a plotted grid, enter the number corresponding\nto the location you wish to place your marker." << endl << endl;

	cout << "1 | 2 | 3" << endl;
	cout << "----------"  << endl;
	cout << "4 | 5 | 6" << endl;
	cout << "----------"  << endl;
	cout << "7 | 8 | 9" << endl << endl << endl << endl;

	cout << "Would you like to play against a friend or the pc? ";
	cin >> todo;
	cout << endl << endl;

		ttt.startUp(todo);
	}
	else
		return 0;
	

};

[Aceix]

Put the class in a header file.
The return type of numtochar should be char since you return only a letter.

Aceix.

[quwackers]

Will do thanks :) is it bad practice to call functions in functions in a class?

[Smac89]

int TicTacToe::pos can only take 9 elements, on line 13 you are storing 10

std::string TicTacToe::numToChar(int) - use switch statements when you start using multiple if/else if http://www-numi.fnal.gov/computing/minossoft/releases/R2.3/WebDocs/Companion/cxx_crib/switch.html

void TicTacToe::printBoard() - it would be less writing if you use a for-loop to print instead

void TicTacToe::startUp(std::string) - line 43 - turns was not initialized before this, so the code might not behave as expected

void TicTacToe::humanMove(int) line 73 - check that the number does not exceed the boundaries of the array first before checking if the value at that position exists. So it should be:
> if(move < 10 && move >= 0 && pos[move] == 0)

[ne555]

`numToChar(int x)' shouldn't be a member function.
`humanMove(int player)' use a loop instead of recursion.

Also, your class is too coupled with the user interface.
By the way, learn to indent.

[quwackers]

@ne555

Do you mean 'numToChar(int x)' instead of calling it inside another function just integrate it with that function?

When you said "your class is toop coupled with the user interface." do you mean is has to many cout's?

And learn to indent?


Sorry I am really knew and haven't quite caught all the termonology yet -___-

[MiiNiPaa]

Do you mean 'numToChar(int x)' instead of calling it inside another function just integrate it with that function?

It means that it should be outside your class:

string numToChar(int x) 
{
    //Code here
}

class TicTacToe
{
    //class definition here
}

And learn to indent?

http://en.wikipedia.org/wiki/Indent_style