Here is a Tic Tac Toe game in C++ that I have been working on. My questions are as follows:

I tested it for bugs, but if you find any, please post the solution!

I want to make the code more recursive, in a sense. Instead of having multiple lines of code that do the same things with different values, it would be nice to compress it even more. You see, I'm a minimalist and I'm lazy (fundamental programmer traits!) so less typing is good.

I'm wondering if my code follows the best common practices, and what those practices might be. For instance, is using camelCase for variables and something_like_this good for functions?

The next thing I noticed was the difference between the user interface (1-based coordinates) and the internal representation (0-based coordinates). This is fine, but your implementation is a bit awkward in this area. You have lines like

x--;y--;print_board(previousPlayer2Piece, x, y);

which is a pretty unusual style. An improvement would be:

print_board(previousPlayer2Piece, x-1, y-1);

A better implementation might be to get the user input into different variables, such as userRow and userCol. So you would have (omitting your input validation and retry logic):

Okay, that sounds great! I was wondering if I could use a for loop to make the checking for win more recursive. Can I do the the same thing in the check_for_overlap function as well? Also, are my naming conventions good?
–
JohnBobSmithJun 24 '14 at 2:02

@Greg Hewgill has offered a better approach to doing this, but I'll mention some best-practice things about the code itself.

The class name tic_tac_toe uses the same naming convention as your functions. It's usually preferred to have separate naming for types. User-defined types are typically named in PascalCase. As such, tic_tac_toe can be renamed as TicTacToe.

This class is essentially just a struct since everything is public. Your three char variables would be considered data members, and they should be private. This will prevent them from being modified outside of the class directly. They're primarily supposed to be maintained within the class. In addition, if you have functions that shouldn't be accessible outside of the class, then they should be private as well.

You should look into replacing that C-style array with a storage container, such as std::array or std::vector. They have many more benefits over C-style arrays, and they'll help keep your code cleaner and less error-prone.

With so much code defined in the class, you should declare your data members and functions within the class and define them outside of it, perhaps in a separate file.

You don't need to explicitly return 0 at the end of main(). The compiler will automatically do this for you as successful termination is implied.

Greg's answer covers the important stuff; I can contribute a few "best practices".

std::cout << "\n"; is not the recommended way to emit a newline; you should use std::cout << std::endl;EDIT: see comment below

I don't see any reason to make board, previousPlayerPiece nor previousPlayer2Piece public. Keep them private since they're only used inside the class.

Some of the comments are not useful. The purpose of member variables is to be used by the methods.

a and b are not informative variable names. Neither are pos1 and pos2 nor x and y which seem to be used for the same purpose. I suggest consistent row and column , perhaps with a prefix or suffix for what they're used for (e.g. playerMoveRow). If you really have to use a single-letter variable, i and j are typically used for generic loop indexes.

Perhaps init_board() should just be the class constructor

print_board() seems to have two functions: display the board, and update it with the player's latest move. I suggest splitting this into two functions

board[pos1][pos2] = playerPiece; Why is this in the inner loop? That makes it get executed 9 times. It doesn't even belong in print_board().

Change the while (true) loop into a do { } while loop. do is for loops that execute at least once.

What is the purpose of the player_piece argument to check_for_overlap()?

Alright snowbody, I will implement all of your suggestions and post a pastebin of the updated code.
–
JohnBobSmithJun 24 '14 at 15:29

I need to clarify something. You should use endl whenever you are done with your output, as it makes sure the output stream is flushed. It's not as necessary to use endl if you're going to send more output immediately (before anything else happens)
–
SnowbodyJun 25 '14 at 14:06