Tic-Tac-Toe optimization 2.0 with AI

Previous question (without AI): Tic-Tac-Toe optimization

This new code has a main file, a base game class, and two derived human and computer classes.

I welcome any suggestions for improving my code. Also, if I'm being needlessly complex on writing the code, let me know. That is my main concern.

Here, I have the welcome screen and I present the player with options. The function setcolor() is defined in game.h, but outside the game class scope, so that I can use it since I've included that header file. It gives the player 3 options: human vs human, comp vs human, and exit.

tic-tac-toeV2.0.cpp

#include "stdafx.h"
#include 
#include 
#include 
#include "game.h"
#include "human.h"
#include "computer.h"
#include 

void main_play();
int main(){

    SetColor(DARKGREEN);
    std::cout>choice;
    if(choice>replay;
    if(replay==1){
        main_play();
    }
    else{
        SetColor(GREEN);
        std::cout<<"Thank You for Playing :)\n\n";
        SetColor(WHITE);
        return;
    }
    }
    SetColor(RED);
    std::cout<<"That is not a Valid Choice....\n Please Make a Valid Choice....\n\n";
    SetColor(WHITE);
    main_play();
}

The game class is the base class and has functions that are necessary for both modes of play.

It has a function for inserting human player's choice on the board. It has another function for checking if there is a win and then displays the appropriate messages.

The win-display function displays the win situation with colored row, col or diagonal, and it uses the color converter and winner functions to do this.

The are_elements_equal() function checks if all the elements in a row, col or diagonal are equal.

game.h

```
#ifndef __GAME_H_INCLUDE
#define __GAME_H_INCLUDE

#include "stdafx.h"
#include
#include
#include
#include
#include
#include
#include

enum win_associations {R1=1,R2,R3,C1,C2,C3,D1,D2};
enum state {WIN=0,DRAW,KEEP_PLAYING};
enum Color { DARKBLUE = 1, DARKGREEN, DARKTEAL, DARK

Don't use double underscore in your identifiers (they are reserved).

Don't start macros with underscore as as identifers in the global namespace starting with an underscore are reserved.

__COMPUTER_H_INCLUDE

Design

I don't like the design:

Game
   Human:     Game
   Computer:  Game

The problem here is that the game logic changes depending on who is playing. Thus if you have a bug in the game logic then it is spread across two classes (Human/Computer).

I would have put all the Game logic into Game.

Then I would have had a class Player that represents the players that are passed to the game on construction. When it is a players turn you call getMove() on the player and it returns the next move. You can then specialize the getMove() for human and computer without compromising the game logic.

class Player
{
    public:
        virtual ~Player();
        virtual  int getMove(BoardState const& board) = 0;
};
class Human
{
        virtual  int getMove(BoardState const& board)
        {
            int  result;

            // Humans not to see board.
            std::cout > result))
            {
                // Bad input.
                // Fix stream.
            }

            return result;
       }
};
class ComputerWithAI_1
{
        virtual  int getMove(BoardState const& board);
};

class Game
{
     public:
        Game(std::unique_ptr p1, std::unique_ptr p2)
            : player1(p1)
            , player2(p2)
        {}
        void play()
        {
            int currentPlayer = 0;
            while(state_of_game()==KEEP_PLAYING)
            {
                int move = ((currentPlayer/%2)?player1:player2)->getMove(board);
                // Validate move etc...
                // Display error message
                // Or update currentPlayer and repeat.
            }
        }
};

More General comments:

@Jamal: has good comments. So I am only including stuff where I disagree with him or he has not covered.

Header files:

#include "stdafx.h"       // has to be first
#include 
#include 
#include 
#include "game.h"
#include "human.h"
#include "computer.h"
#include 

They should be ordered: Most specific first to most general last. I order mine like this:

// This is Class.cpp
#include "Class.h"
#include "OtherMyClassIdependon1.h"
#include "OtherMyClassIdependon1.h"

// Now C++ header files  // Lots of people order these alphabetically
#include         // Personally I group them
                         // All the containers together.
                         // All the streams together.
                         // etc. Each group alphabetical.

// Now C header files    // Lots of people order these alphabetically
#include 

// Now System header files
#include 

So in your case I would have done:

#include "stdafx.h"       // has to be first
#include "game.h"
#include "human.h"
#include "computer.h"
// C++
#include 
#include 
#include 
// C Header
// System Headers
#include 

Common Code refactoring:

void game::Winner(char c){
    if(c=='O'){
            SetColor(YELLOW);                     // This is common to both conditions
            std::cout<<player1<<" Won....\n";
            SetColor(WHITE);                     // This is common to both 
        }
        else if(c=='X'){
            SetColor(YELLOW);                     // This is common to both 
            std::cout<<player2<<" Won....\n";
            SetColor(WHITE);                     // This is common to both 
        }
}

It would be easier to re-write as:

void game::Winner(char c)
{
    SetColor(YELLOW);
    if(c=='O') {
        std::cout<<player1<<" Won....\n";
    }
    else if(c=='X') {
        std::cout<<player2<<" Won....\n";
    }
    SetColor(WHITE); 
}

The function void game::win_display() is a bit convoluted. You are basically doing the same thing 8 times. Just adjusting which elements are displayed in a winning colour. You could re-write like this:

void game::win_display()
{
    bool h1,h2,h3,v1,v2,v3,d1,d2; // Init as appropriate

    std::cout
        <<" "<<Win(h1||v1||d1)<<board[0]<<"  | "<<Win(h1||v2        )<<board[1]<<"  | "<<Win(h1||v3||d2)<<board[2]
        <<"  \n____|____|____\n"
        <<" "<<Win(h2||v1    )<<board[3]<<"  | "<<Win(h2||v2||d1||d2)<<board[4]<<"  | "<<Win(h2||v3    )<<board[5]
        <<"  \n____|____|____\n"
        <<" "<<Win(h3||v1||d2)<<board[6]<<"  | "<<Win(h3||v2        )<<board[7]<<"  | "<<Win(h3||v3||d1)<<board[8]
        <<"  \n    |    |    \n\n";
    Winner(*row1[0]);
}

Now we just have to write the stream manipulator win() to set up the colours for the next element to be printed and set them back to WHITE after the element is printed.

This is a nice trick for stream manipulators:

```
struct Win
{
Win(bool x): good(x), sptr(NULL) {}
Win(Win const& c, std::ostream& s): good(c.good), sptr(&s) {}
bool good;
std::ostream* sptr; // No ownershi

StackExchange Code Review Q#30238, answer score: 8