Tic Tac Toe with classes

Question

I have created Tic Tac Toe in c++ using classes. Is this efficient code?

And GNU GCC compiler gives these warnings:

Warning: control reaches end of non-void function [-Wreturn-type]
Warning: extended initializer lists only available with -std=c++11 or -std=gnu++11 [enabled by default]
warning: non-static data member initializers only available with -std=c++11 or -std=gnu++11 [enabled by default]

TicTacToe.h:

#ifndef TICTACTOE_H_INCLUDED
#define TICTACTOE_H_INCLUDED

class TicTacToe
{
public:
    TicTacToe();
    ~TicTacToe();
    void welcome();
    const  void getInput();
private:
    char blockEntries[9] = {'1', '2', '3', '4', '5', '6', '7', '8', '9'};
    int checkStatus() const;
    int turns = 10;
    void printBoard() const;
    const void resetBoard();
 };

#endif // TICTACTOE_H_INCLUDED

TicTacToe.cpp:

#include <iostream>         // Standard Input output stream
#include "TicTacToe.h"      // Game class Header file
#include <conio.h>
#include <iomanip>

TicTacToe::TicTacToe()      // Default Constructor
{
}
void TicTacToe::welcome()   // Shows Welcome message
{
    std::cout << "      ***********************************************************" << std::endl;
    std::cout << "      ***                                                     ***" << std::endl;
    std::cout << "      ***        Welcome to Tic Tac Toe (MultiPlayer)         ***" << std::endl;
    std::cout << "      ***                                                     ***" << std::endl;
    std::cout << "      ***********************************************************" << std::endl << std::endl;
    printBoard();           // print game board
}
TicTacToe::~TicTacToe()
{

}
const void TicTacToe::getInput()  // get input from user and find winner
{
    char sign = 'X';
    int input = 0;
    int player = 1;

    do
    {

        player = (player % 2) ? 1:2;   // Alternate user
        std::cout << "Player" << player << " turn: ";
        std::cin >> input;                   // input array numbers
        if((input >= 1 && input <= 9) && ( blockEntries[input - 1] != 'X' && blockEntries[input - 1] != 'O'))
        {
            std::cout << std::endl;
            sign = (player == 1) ? 'X' : 'O';
            blockEntries[input - 1] = sign;
            printBoard();
            if (checkStatus() == 1)
            {
                std::cout << "------------------------------------\n";
                std::cout << "==========> Player" << player << " Wins! <=========\n";
                std::cout << "------------------------------------\n\n\n\n";
                turns = 0;
                resetBoard();

            }

            turns = turns - 1;
            player++;
        }
        else if(input == 0)
            turns = 0;
        else
        {
            std::cout<<"Invalid move ";
            std::cin.ignore();
            std::cout << std::endl;
            //cin.get();
        }

    }

    while(turns > 1);
    if(turns == 1)
    {
        std::cout << "------------------------------------\n";
        std::cout << "====> Player1 and Player2 Tie <=====\n";
        std::cout << "------------------------------------\n\n\n\n";
        resetBoard();

    }


}
 void TicTacToe::printBoard() const
{
    std::cout << "                 player1 ---> X        Player2 ---> O" << std::endl << std::endl;
    std::cout << std::setw(70) << "Enter 0 to terminate it." << std::endl;
    std::cout << "        |     |        " << std::endl;
    std::cout << "     " << blockEntries[0] << "  |  " << blockEntries[1] << "  |  " << blockEntries[2] << std::endl;
    std::cout << "   _____|_____|_____   " << std::endl;
    std::cout << "        |     |        " << std::endl;
    std::cout << "     " << blockEntries[3] << "  |  " << blockEntries[4] << "  |  " << blockEntries[5] << std::endl;
    std::cout << "   _____|_____|_____   " << std::endl;
    std::cout << "        |     |        " << std::endl;
    std::cout << "     " << blockEntries[6] << "  |  " << blockEntries[7] << "  |  " << blockEntries[8] << std::endl << std::endl;

}
int TicTacToe::checkStatus() const     // check status of the game
{
    if (((blockEntries[0] == blockEntries[1] && blockEntries[1] == blockEntries[2] ) ||
        (blockEntries[0] == blockEntries[3] && blockEntries[3] == blockEntries[6] ) ||
        (blockEntries[6] == blockEntries[7] && blockEntries[7] == blockEntries[8] ) ||
        (blockEntries[2] == blockEntries[5] && blockEntries[5] == blockEntries[8] ) ||
        (blockEntries[0] == blockEntries[4] && blockEntries[4] == blockEntries[8] ) ||
        (blockEntries[2] == blockEntries[4] && blockEntries[4] == blockEntries[6] ) ||
        (blockEntries[1] == blockEntries[4] && blockEntries[4] == blockEntries[7] ) ||
     (blockEntries[3] == blockEntries[4] && blockEntries[4] == blockEntries[5] )) &&
        (turns > 1)
       )
        {
            return 1;
        }
}
const void TicTacToe::resetBoard()
{
    int values = 49; // '1' has value 49
    for(int i = 0; i <=9; i++ )
    {
        blockEntries[i] = values;
        values += 1;
    }
}

This is the main function calling all of the functions:

#include <iostream>
#include "TicTacToe.h"

int main()
{
    char restart;
    TicTacToe game;
    game.welcome();
    game.getInput();
    std::cout << "Do you want to play again? Enter (y/n): ";
    std::cin >> restart;
    if (restart == 'y')
    {
        std::cout << "\n\n\n";
        game.welcome();
        game.getInput();
    }
    else if (restart == 'n')
        return 0;
}

Answer 1

1. In general you should keep separate the logic of your application from the user interface. So your TicTacToe object should represent the state of the game (player turn, content of the board) and heave methods to play a move (and validate it), and inspect the status. In this simple program I would prefer to keep the input/output code inside main or in some helper function.

2. Since your constructor and destructor do nothing, don't declare them. You spare to write more code, you don't need to comment them being 'default' and the user looking at the *.h file has something less to care about.

3. Your welcome function should accept the output stream as an argument. This shortens the function itself (you can use out instead of std::cout) and make it more general. In fact it is good to use std::cout only in your main function. The same is true for the getInput function which should have both input and output streams as parameters. Anyway, as already said both function should not be part of the class implementing the logic of the game... In particular getInput is the main loop of the program, which could well be in a function outside the TicTacToe class.

4. The printBoard function is actually good to have also if TicTacToe only represents the logic of the game, because it can be used to have a textual representation of the object. However you should implement it overloading the friend operator<<

5. Even if your board is very small (3x3) it is better to use loops instead of repeating the code when you iterate over the rows. This makes the code a little bit shorter but is much more maintainable when you want to make modifications. This is true in both printBoard and checkStatus.

6. the name of the variable turns is misleading. If you represent the number of turns you expect it to increase. Instead you are maybe representing the number of empty cells. It is very difficult to understand if the check of the do...while i.e. turns>1 is correct or is off by one.

7. the variable player should only have values 1 and 0. So you should make the modulus check after increasing the value, not at the beginning of the following loop. This will preserve the invariant of the loop.

8. filling the board with the numbers representing possible moves is something related to the output functions not the logic of the game. Better to have only three possible values for EMPTY, PLAYER1, PLAYER2 representing the content of the cells. You are using the fact that empty cells are different each other in the checkStatus function. This is very smart, but not good if you want the code to be easy to understand and easy to maintain. If someone would like to implement a graphic interface he will not need to put numbers in the cells, but will run the risk to break the code if it tries to modify it.

9. conio.h is a non standard header. The function checkStatus should return 0 (this is causing the warning).

Answer 2

This isn't really "classes" it is one class for the entire game and every aspect of it, and a main function that drives it.

There are 8 winning "lines" on the board which, if they all hold the same value and are not empty, is a win to the player whose value occupies each position in the line.

Make a line into a class.

That class can check its status.

Each cell could be a class and from the cell would be able to find all the lines it is contained in. That could be a method and generated "on the fly" or could be cached within each cell.

Thus when the user plays into a cell you'd only need to check the lines within it to see if they have played a winning move. Only the one who last played can have made a winning move.

There are 4 lines for the middle cell, 3 for a corner and only 2 for an edge.