TicTacToe Implementation With Classes

Question

This is my first program using a class that I personally designed. Please pay special attention to the design of the class and I would love pointers and advice on how to better improve logical design of classes (what should be grouped with them, better ways of creating classes to interact with each other, etc).

I am also aware that there are constructors and deconstructors, but i'm unsure when I should use those or how they should be implemented. Advice for those would be helpful as well! I recently decided to stop using using namespace std; as i've read there are many dangers with this practice and frankly...writing std:: isn't all that difficult. If someone has a counterargument to this I would love to hear it as I am a beginner and could be very wrong.

Main

//implementation of TicTacToe
//Using classes this time
#include <iostream>
#include "TicTacToeClass.h"
int main()
{
 //Assumes no play unless user decides they want to play and initializes game variable to TicTacToe class
 bool play = false;
 TicTacToe game;
 play = game.getUserWantToPlay();
 //allows for multiple games to be played
 while(play == true)
 {
 char playerWinner = 'n';
 char squareArray[9] = {'1', '2', '3', '4', '5', '6', '7', '8', '9'};
 char player = 'X';
 //single game iteration
 while(playerWinner == 'n')
 {
 game.drawBoard(squareArray);
 game.getPlayerMove(squareArray, player);
 playerWinner = game.checkForWin(squareArray, player);
 if(playerWinner == 'n')
 {
 player = game.togglePlayer(player);
 }
 }
 game.drawBoard(squareArray);
 play = game.getUserWantToPlay();
 }
 return(0);
}

Class Implementation

//TicTacToe class implementation
//Leeroy Jenkins
#include "TicTacToeClass.h"
#include <iostream>
bool TicTacToe::getUserWantToPlay()
{
 char response;
 bool invalidResponse = true;
 bool play = false;
 while(invalidResponse == true)
 {
 std::cout << "Would you like to play a new game of TicTacToe? (y/n) " << std::endl;
 std::cin >> response;
 if(response == 'y')
 {
 invalidResponse = false;
 play = true;
 }
 else if(response == 'n')
 {
 std::cout << "No Problem!";
 invalidResponse = false;
 }
 else
 {
 std::cout << "Please input a proper response (y/n)" << std::endl;
 }
 }
 return play;
}
void TicTacToe::drawBoard(char boardArray[])
{
 //draws the game board with updated characters for each player
 std::cout << "Player 1 (X) - Player 2 (O)" << std::endl << std::endl << std::endl;
 std::cout << " | |" << std::endl;
 std::cout << " " << boardArray[0] << " | " << boardArray[1] << " | " << boardArray[2] << std::endl;
 std::cout << "____|_____|____" << std::endl;
 std::cout << " | | " << std::endl;
 std::cout << " " << boardArray[3] << " | " << boardArray[4] << " | " << boardArray[5] << std::endl;
 std::cout << "____|_____|____" << std::endl;
 std::cout << " | | " << std::endl;
 std::cout << " " << boardArray[6] << " | " << boardArray[7] << " | " << boardArray[8] << std::endl;
}
void TicTacToe::getPlayerMove(char boardArray[], char player)
{
 //Gets player move and stores in board array for display through next iteration
 bool playerMoveFound = false;
 char playerTurn = '0';
 char playerMove = '0';
 if(player == 'X')
 {
 playerTurn = '1';
 }
 else
 {
 playerTurn = '2';
 }
 while(playerMoveFound == false)
 {
 std::cout << "Player " << playerTurn << " please make a move" << std::endl;
 std::cin >> playerMove;
 for(int x = 0; x < 9; x++)
 {
 //If finds the array number makes the change to the iteration...prevents x or o movement
 if(playerMove == boardArray[x] && playerMove != 'X' && playerMove != 'O' && playerMove != 'x' && playerMove != 'o')
 {
 boardArray[x] = player;
 playerMoveFound = true;
 }
 }
 if(playerMoveFound == false)
 {
 std::cout << "Invalid player move..." << std::endl;
 }
 }
}
char TicTacToe::checkForWin(char boardArray[], char player)
{
 char playerWin = 'n';
 int testForTie = 0;
 //Tests winning combinations
 if(boardArray[0] == boardArray[1] && boardArray[1] == boardArray[2])
 {
 playerWin = player;
 }
 else if(boardArray[0] == boardArray[3] && boardArray[3] == boardArray[6])
 {
 playerWin = player;
 }
 else if(boardArray[0] == boardArray[4] && boardArray[4] == boardArray[8])
 {
 playerWin = player;
 }
 else if(boardArray[1] == boardArray[4] && boardArray[4] == boardArray[7])
 {
 playerWin = player;
 }
 else if(boardArray[2] == boardArray[4] && boardArray[4] == boardArray[6])
 {
 playerWin = player;
 }
 else if(boardArray[2] == boardArray[5] && boardArray[5] == boardArray[8])
 {
 playerWin = player;
 }
 else if(boardArray[3] == boardArray[4] && boardArray[4] == boardArray[5])
 {
 playerWin = player;
 }
 else if(boardArray[6] == boardArray[7] && boardArray[7] == boardArray[8])
 {
 playerWin = player;
 }
 else
 {
 //Tests for a tie game
 for(int x = 0; x < 9; x++)
 {
 if(boardArray[x] == 'x' || boardArray[x] == 'o' || boardArray[x] == 'X' || boardArray[x] == 'O')
 {
 testForTie++;
 }
 }
 if(testForTie == 9)
 {
 playerWin = 't';
 }
 }
 if(playerWin == player)
 {
 if(player == 'X')
 {
 std::cout << std::endl << "Congratulations player 1! You Win!" << std::endl;
 }
 else
 {
 std::cout << std::endl << "Congratulations player 2! You Win!" << std::endl;
 }
 }
 else if(playerWin == 't')
 {
 std::cout << "Tie! You should play again to settle the duel!" << std::endl;
 }
 return(playerWin);
}
char TicTacToe::togglePlayer(char player)
{
 player = player == 'X' ? 'O':'X';
 return(player);
}

Class Header

/*
 * TicTacToeClass.h
 *
 * Created on: Jun 15, 2016
 */
#ifndef TICTACTOECLASS_H_
#define TICTACTOECLASS_H_
class TicTacToe
{
 public:
 bool getUserWantToPlay();
 void drawBoard(char boardArray[]);
 void getPlayerMove(char boardArray[], char player);
 char togglePlayer(char player);
 char checkForWin(char boardArray[], char player);
};
#endif /* TICTACTOECLASS_H_ */

• Mathieu Guindon: I'm pretty sure "Stop using namespace std;!" would have been the first low-hanging fruit piece of feedback you would have received. Any counter-argument advocating its use is inherently flawed, good job killing bad habits early!
• syb0rg: Hello! Please don't make changes to the original post once it has been reviewed, as that invalidates the current answers. Please see our meta side on performing iterative reviews for more information!
• StormsEdge: @syb0rg Apologies! Forsavir just informed me of that. I will put it back to its original state and do it properly. I apologize!
• syb0rg: @StormsEdge I've already done that for you, no worries

Answer 1

Some initial thoughts...

The board

It seems a little odd that the board array is being passed into the TicTacToe class. I think if you really want to inject it as a dependency into the TicTacToe class, then you'd be better off wrapping it in a class. Alternately, the simpler option would be to make the board a private member variable of your class, which it then has complete control over. As it stands, both your main and your TicTacToe class have intimate knowledge about the array and it's size, although the class doesn't validate this in anyway, so if the caller creates an array that's too small things will start going wrong.

Checking for tie

When you're checking for a tie, you're checking for upper and lower case players ('X' and 'x'). If it wasn't for the fact that the array is being supplied you'd be guaranteed that 'x' wouldn't occur, since you only insert 'X's into the grid. You do the same 'X' or 'x' check when allowing the player to make a move.

Winning condition

When you're checking for a win, you're trusting the caller to pass in the correct player ('X' or 'O'). If the last player to take a move was 'X' and they placed a winning move, but checkForWin was called with player 'O', it would declare 'O' the winner, even though there is actually a line of 'X'.

Checking for line wins (horizontal + vertical) is easily modelled with loops, which will help to simplify your checkForWin method. I'd also suggest taking the winning player from the board, rather than from the supplied parameter.

Prompting

You convert 'X' to 1 before prompting the player to make a move. Why not just have it be X's move?

std::cout << "Player " << playerTurn << " please make a move" << std::endl;

Variable Naming

In your playerMove method, you're using 'x' as an iterator variable:

for(int x = 0; x < 9; x++)

Given that 'x' actually has some meaning in your game, I'd consider using a different name, even if it was just 'i'.

• StormsEdge: thanks for the thoughts! Any thoughts about class constructors or deconstructors? Also perhaps a very ignorant beginner question, but if I create a private member variable; am I able to display it using std::cout?
• forsvarir: @StormsEdge You can pass member variables to other classes/methods, but they can't access it directly. So, if playerTurn for example was a member variable, you would be able to use it in calls to cout from within the TicTacToe class, but you wouldn't be able to use it in calls from your main method. The access modifiers change the visibility, so the compiler will guide you as to what will and won't work (it won't compile if you try to access something that you can't, you don't need to worry about the code compiling and then failing when you run it because something is private).
• forsvarir: @StormsEdge destructors are used to clean up after class, mostly you're concerned about closing resources (such as files), or freeing allocated memory etc that you don't want to hang around after the class has been destroyed. You don't need to do things like reset normal member variables to 0 etc. So, if you were to dynamically create the board using new in your constructor then you would need to make sure the memory was cleaned up in your destructor (although if you use the various smart pointer, such as unique_ptr and shared_ptr classes it will mostly happen for you).
• forsvarir: @StormsEdge If you haven't already, then you might want to have a look at this alternative implementation: codereview.stackexchange.com/q/78536/4203 which uses the constructor to initialise the board. It still doesn't have a destructor though, as it doesn't allocate any resources that need to be cleaned up.
• forsvarir: @StormsEdge I've had a look at your edit, which mostly moves the squareArray into the class. It looks like an improvement and you're using the constructor correctly. With modern C++ compilers, you can just initialise the squareArray directly in the class definition char squareArray[9]{'1','2','3','4','5','6','7','8','9'}; but either approach works.