Console Based Tic-Tac-Toe in C++

I haven't done a deep analysis of your code, and I haven't tried to run it (because it won't run on my system). There is a lot here that needs work to meet modern C++ standards. Let's go section-by-section and see how far we get.

Headers

#include <time.h> 

C headers of the form <xxx.h> are deprecated in C++. In almost all cases, you can simply do <cxxx>.

So here's you'd do #include <ctime>.

Global variables

Your whole program works via global variables. This is a terrible design, for numerous reasons. To give just one example: the global variables w and h are only used in a single function - drawBoard() - but they are visible to the entire program. It would be very easy for some other function to accidentally change the value of w or h (one letter variable names are also a bad idea!), and the next time you try to draw the board, it could be truncated, it could draw gibberish, or your program could crash.

All of those global variables should be removed, and every function should be refactored to take arguments, and return results. You don't need a single global variable in your program.

The board

You have defined the game board like this:

char numbers[3][3] = { {'1', '2', '3'}, {'4', '5', '6'}, {'7', '8', '9'} }; 

The first problem, already mentioned, is that this is a global variable. In addition to all the usual problems with global variables, by making the board global, you make it harder to do a lot of interesting things. It will be harder to "play again", if the user enjoyed the game. It will be harder to undo moves, if you want to add that feature. It will be harder to code an AI to make smart moves (rather than just random plays).

C++ is a strongly typed language; it's all about the types. If you get your types right... everything else just works.

What you should do is make a board class. A properly designed board class could initialize itself, and do checks to make sure you don't play out of bounds or play a square that's already been played, and so on.

But that's a big change. Let's start with small improvements for now.

For now, rather than using C arrays for the board, use proper C++ arrays. And while we're at it, define the board type:

using board_type = std::array<std::array<char, 3>, 3>; board_type numbers = {{ {'1', '2', '3'}, {'4', '5', '6'}, {'7', '8', '9'} }}; 

Just this change makes your code safer (C arrays have dangerous properties, like silent decay to pointers). Other benefits will be clear soon.

drawBoard()

First, as with all of your functions, you should modify it to avoid global variables. In this case, you'd need to take the board to draw as a parameter. Something like this:

void drawBoard(board_type const& numbers) { 

Even better, rather that always writing to std::cout, you can take the output as a parameter:

void drawBoard(board_type const& numbers, std::ostream& out) { 

If you do that, you can even write boards to files, for logging/debugging, or saving games.

This function doesn't need to return anything but it does need to use w and h. There's no reason those can't be local variables, though - they're not even used anywhere else.

Even better, you could calculate w and h from the board dimensions. You could also make them const, because they won't change. For example:

auto const w = numbers[0].size() * 6; auto const h = numbers.size() * 6; 

Next:

system("cls"); 

First, this should probably be std::system("cls");. Second, you should be aware that using std::system() is generally unportable. In fact, I couldn't run your code if I wanted to, because my system doesn't have "cls".

From this and other clues, I guess you're probably trying this program in a Windows GUI. If that's the only place you want the program to work, that's fine, but if you want portable code, you can't use system-specific stuff, or assume that the console will exit after your program is done (or even that there is a console).

std::cout << std::endl; 

Using std::endl is almost always wrong. If all you're doing is writing a new line, just do std::cout << '\n';. It can be hundreds of times faster.

You should especially never do std::cout << std::endl << std::endl;. That should almost always be std::cout << '\n' << '\n'; or std::cout << "\n\n";.

playerInput()

Once again, to avoid global variables, you could take board as a reference parameter:

void playerInput(board_type& numbers) 

Even better, you could make it work for either X or O:

void playerInput(board_type& numbers, char play) 

The main problem with this function is that it does too much stuff. It reads a number in from cin, it checks to see if it's a valid move, and then it does the move. That's 3 different jobs, that should be 3 different functions, like this.

void playerInput(board_type& numbers, char play) { while (true) { // Read number from cin and make sure it's in range auto numInput = getPlayerMove(); // Check that number is available to play if (isValidMove(numbers, numInput)) { // Make the play and return. makeMove(numbers, numInput, play); return; } // If we get here, the number was not available to play, // so print the error message, and the loop will start over std::cout << "Invalid input.\n\n"; } } 

If you did split out those 3 functions, they would be reusable elsewhere. For example, when the AI is making a move, it could reuse isValidMove() and makeMove() functions.

As for the function as it's currently written....

int numInput; bool numFound = false; 

Declaring all your variables at the start of the function is archaic C practice. In C++ (and C after 1999), declare your variables when they're needed.

std::cin >> numInput; while (std::cin.fail()) { drawBoard(); std::cin.clear(); std::cin.ignore(); std::cout << "Invalid input." << std::endl << std::endl; std::cout << "Select a box to mark: "; std::cin >> numInput; } 

This could be simplified by taking advantage of istream's conversion to bool, like this:

int numInput = 0; while (!(std::cin >> numInput)) { drawBoard(); // or perhaps std::cin.clear(); std::cin.ignore(); std::cout << "Invalid input.\n\n"; std::cout << "Select a box to mark: "; } 

This is technically not a portable way to convert a character to a digit:

int charInt = numbers[i][j] - '0'; 

But I've never used a platform where it fails. However, it does cause a bug in your code because not everything in numbers is a digit - some characters are X or O. To see the bug in action, start a game, play one good move, then on your second move, enter either 40 or 31.

if (numInput == charInt) { numbers[i][j] = 'X'; drawBoard(); numFound = true; break; } 

The only purpose of numFound in this function is to break out of nested loops. Instead, once the number is found, you can just return.

else if (j == 2 && i == 2) { std::cout << "Invalid input." << std::endl << std::endl; playerInput(); } 

Using recursion to handle errors like this is unwise. In an unbounded recursion, it's possible to get stack overflow. (Although in this case, the compiler might be able to use a tail call to avoid nested stacks.) This function should be refactored to use a loop, not recursion.

compRandInput()

As always, this function should take parameters and return values to avoid using global variables.

Don't use rand(). It can be a garbage implementation, and it's hard to use correctly. (In fact, you're using it wrong.) Use the <random> library.

Once again, this recursion is unbounded, so you should probably use iteration instead.

void compRandInput(board_type& numbers, char play, random_engine_type& rnd_eng) { auto col_dist = std::uniform_int_distribution<>{0, numbers[0].size() - 1}; auto row_dist = std::uniform_int_distribution<>{0, numbers.size() - 1}; int row = 0; int col = 0 do { row = row_dist(rnd_eng); col = col_dist(rnd_eng); } while (numbers[row][column] == 'X' || numbers[row][column] == 'O'); // Or, even better, reuse the function: // while (!isValidMove.... numbers[row][col] = play; } 

And so on...

That should be enough to keep you busy refactoring for a while. The most important recommendations are:

• Make a class for the board. That class can do all the busywork to keep things sane and avoid bugs.
• Don't use global variables. Have your functions take parameters and return values instead.
• Break tasks into functions, and reuse them. Each function should have one job; functions that do multiple things should be broken up. Look especially for opportunities to reuse code. For example, whether it's a human player or AI player, there are a ton of things that need to be done for both, like checking whether a move is valid, actually making a move, and checking for a win.