This can cause name collisions because it adds every name in the std namespace to the global namespace. For a small program like this one it's unlikely that you'll run into any problems (then again, maybe not) but it's best to get into the habit of using the std:: prefix on names in the std namespace.
Alternatively, you can introduce using declarations like using std::cout; to add specific names to the global namespace.
Avoid std::endl in favor of \n
std::endl flushes the stream, which can cause an unnecessary loss in performance. In printBoard() you use std::endl three times but you can easily replace some of those uses with \n.
Use a class for the Tic Tac Toe board
Instead of using a 3x3 array it's better to create a class to hold the state of the Tic Tac Toe board, with functions to set or get the value of a place on the board, determine whether or not the board is filled, etc. This allows you to hide implementation details which may change later on -- for example, if your compiler supports C++11 or will do so in the future you may want to change your internal representation of the board to use std::array. If you have a class for the Tic Tac Toe board the external interface would not change, only the code that implements the functionality.
Here's what that class might look like:
class Board {
int board[3][3];
public:
typedef int value_type;
// More modern alternative to typedef:
//using value_type = int;
Board() {
for (std::size_t i = 0; i < 3; i++) {
for (std::size_t j = 0; j < 3; j++) {
board[i][j] = 0;
}
}
}
// Gets the value of a board place
int place(std::size_t x, std::size_t y) const {
// Might want to check coordinates and throw an exception (e.g. std::out_of_range) if coordinate(s) are invalid
return board[x][y];
}
// Sets the value of a board place
// It would probably be better to throw an exception (e.g. std::out_of_range) instead of printing to std::cout
bool place(std::size_t x, std::size_t y, int value) {
if (x > 2 || x < 0) {
cout << "Invalid x coordinate. Try again.\n";
return false;
} else if (y > 2 || y < 0) {
cout << "Invalid y coordinate. Try again.\n";
return false;
} else if (board[x][y] != 0) {
cout << "Place is already filled. Try again.\n";
return false;
} else {
board[x][y] = value;
return true;
}
}
bool is_filled() const {
for (int i = 0; i < 3; i++) {
for (int j = 0; j < 3; j++) {
// Return immediately once we find a non-filled place
if (board[i][j] == 0) return false;
}
}
// All places were found to be filled
return true;
}
};
(Note the use of std::size_t for the coordinate type and the typedef for the int value type, which mimic practices from the standard library.)
You would no longer need initBoard() since the Board constructor performs the initialization, and some of your other functions would become member functions of the class. In some cases you can improve the performance of these functions -- e.g. you can keep a count of how many places are filled by incrementing the count each time a new place is filled by Board::place(), and then Board::is_filled() simply has to check if that count equals 9 rather than checking every place in the board (the performance gain is negligible for such a tiny board but the principle is important).
Overload operator<< for printing the board
printBoard() only prints to std::cout. You can overload operator<< in order to insert the board into any std::ostream. Using the above Board class the code would look like this:
std::ostream& operator<<(std::ostream& os, Board board) {
os << "-----\n";
for (int i = 0; i < 3; i++) {
for (int j = 0; j < 3; j++) {
os << board.place(i, j) << " ";
}
os << '\n';
}
os << "-----" << std::endl;
return os;
}
(Note the replacement of some of the std::endl uses with '\n`.)
To print the board to std::cout you'd simply use std::cout << board;, and you can use the same function to print the board to a different std::ostream such as a std::fstream (for writing to a file).
Make your code DRYer
In some cases you can make your code DRYer. For example, the code in the if and else sections is very similar here:
if (playerOnesTurn) {
cout << "Player 1's turn" << endl;
do {
cout << "Enter x coordinate: "; cin >> x;
cout << "Enter y coordinate: "; cin >> y;
} while (!fillPlace(board, x, y, 1));
} else {
cout << "Player 2's turn" << endl;
do {
cout << "Enter x coordinate: "; cin >> x;
cout << "Enter y coordinate: "; cin >> y;
} while (!fillPlace(board, x, y, 2));
}
You can put that in a function like this:
// Capture Board by reference so we modify the original Board, not a copy
void player_move(Board& board, int player) {
cout << "Player " << player << "'s turn" << endl;
std::size_t x, y;
do {
cout << "Enter x coordinate: "; cin >> x;
cout << "Enter y coordinate: "; cin >> y;
} while (!board.place(x, y, player));
}
Here's a demo program with some of the changes I suggested:
#include <iostream>
#include <ostream>
using namespace std;
class Board {
int board[3][3];
public:
typedef int value_type;
// More modern alternative to typedef:
//using value_type = int;
Board() {
for (std::size_t i = 0; i < 3; i++) {
for (std::size_t j = 0; j < 3; j++) {
board[i][j] = 0;
}
}
}
// Gets the value of a board place
int place(std::size_t x, std::size_t y) const {
// Might want to check coordinates and throw an exception (e.g. std::out_of_range) if coordinate(s) are invalid
return board[x][y];
}
// Sets the value of a board place
// It would probably be better to throw an exception (e.g. std::out_of_range) instead of printing to std::cout
bool place(std::size_t x, std::size_t y, int value) {
if (x > 2 || x < 0) {
cout << "Invalid x coordinate. Try again.\n";
return false;
} else if (y > 2 || y < 0) {
cout << "Invalid y coordinate. Try again.\n";
return false;
} else if (board[x][y] != 0) {
cout << "Place is already filled. Try again.\n";
return false;
} else {
board[x][y] = value;
return true;
}
}
bool is_filled() const {
for (int i = 0; i < 3; i++) {
for (int j = 0; j < 3; j++) {
// Return immediately once we find a non-filled place
if (board[i][j] == 0) return false;
}
}
// All places were found to be filled
return true;
}
};
bool checkWin(Board& board, int player) {
// horizontal
for (int i = 0; i < 3; i++) {
if (board.place(i, 0) == player && board.place(i, 1) == player && board.place(i, 2) == player) return true;
}
// vertical
for (int i = 0; i < 3; i++) {
if (board.place(0, i) == player && board.place(1, i) == player && board.place(2, i) == player) return true;
}
// diagonal
if ((board.place(0, 0) == player && board.place(1, 1) == player && board.place(2, 2) == player) ||
(board.place(0, 2) == player && board.place(1, 1) == player && board.place(2, 0) == player))
return true;
return false;
}
// TODO implement function if the game is a tie before the last move
std::ostream& operator<<(std::ostream& os, Board board) {
os << "-----\n";
for (int i = 0; i < 3; i++) {
for (int j = 0; j < 3; j++) {
os << board.place(i, j) << " ";
}
os << '\n';
}
os << "-----" << std::endl;
return os;
}
// Capture Board by reference so we modify the original Board, not a copy
void player_move(Board& board, int player) {
cout << "Player " << player << "'s turn" << endl;
std::size_t x, y;
do {
cout << "Enter x coordinate: "; cin >> x;
cout << "Enter y coordinate: "; cin >> y;
} while (!board.place(x, y, player));
}
int main() {
Board board;
std::cout << board;
// should be a bool, not an int
// alternatively, just use an int which is 1 when it's player 1's turn and 2 when player 2's turn
bool playerOnesTurn = true;
while (true) {
player_move(board, playerOnesTurn ? 1 : 2);
playerOnesTurn = !playerOnesTurn;
if (board.is_filled()) {
cout << "The game is a tie.\n";
break;
}
std::cout << board;
if (checkWin(board, 1)) {
cout << "Player 1 has won\n";
break;
} else if (checkWin(board, 2)) {
cout << "Player 2 has won\n";
break;
}
}
return 0;
}
I left a few other minor tips in the comments and I didn't make all the changes I suggested (e.g. I did not remove using namespace std).
