4
\$\begingroup\$

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 <iostream>
#include <string>
#include <Windows.h>
#include "game.h"
#include "human.h"
#include "computer.h"
#include <sstream>

void main_play();
int main(){

    SetColor(DARKGREEN);
    std::cout<<"~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~Tic-Tac-Taoe~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"<<std::endl;
    std::cout<<"*By Default Player 1 is 'O' and Player 2 is 'X'\n";
    std::cout<<"*The choice of cells is mapped out in the following legend\n\n\n";
    std::cout<<" 1  | 2  | 3  \n____|____|____\n 4  | 5  | 6  \n____|____|____\n 7  | 8  | 9  \n    |    |    \n\n";
    std::cout<<"********************************************************************************\n\n\n";
    SetColor(WHITE);

    main_play();
    return 0;
}

void main_play(){
    int choice,replay;
    std::cout<<"What do You want to Do?\n 1.Play Human vs. Human(2 Player) \n 2.Computer vs. Human(1 Player)\n 3.Exit\n\n";
    std::cin>>choice;
    if(choice<4){
    switch(choice){
    case 1:
        {
        human two_player;
        two_player.play();
        break;
        }

    case 2:
        {
        computer one_player;
        one_player.play();
        break;
        }
    case 3:
        {
        SetColor(GREEN);
        std::cout<<"Thank You for Playing :)\n\n";
        SetColor(WHITE);
        return;
        }
    }

    SetColor(DARKTEAL);
    std::cout<<"Do You Want to Play Another Game?\n 1.Yes \n 2.No \n";
    SetColor(WHITE);
    std::cin>>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 <iostream>
#include <string>
#include <time.h>
#include <map>
#include <Windows.h>
#include <array>
#include <sstream>

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, DARKRED, DARKPINK, DARKYELLOW, GRAY, DARKGRAY, BLUE, GREEN, TEAL, RED, PINK, YELLOW, WHITE };
void SetColor(Color c);

class game
{
public:
    game();
    bool is_the_move_valid(int);
    bool are_elements_equal(std::array<char*,3>);
    state state_of_game();
    void display_board();
    void win_display();
    void color_conv(char);
    void Winner(char);
protected:
    std::string player1,player2,dummy;
    char board[9];
    char* p;
    std::array<char*,3> row1;
    std::array<char*,3> row2;
    std::array<char*,3> row3;
    std::array<char*,3> col1;
    std::array<char*,3> col2;
    std::array<char*,3> col3;
    std::array<char*,3> diag1;
    std::array<char*,3> diag2; 
    bool board_full();

};

#endif

game.cpp

#include "stdafx.h"
#include "game.h"
#include <sstream>
HANDLE hCon;
void SetColor(Color c){
        if(hCon == NULL)
                hCon = GetStdHandle(STD_OUTPUT_HANDLE);
        SetConsoleTextAttribute(hCon, c);}

//constructor for the class game
game::game(){

    //This fills up the board array with '-' a filler character, also it looks good :)
    for(int k=0;k<9;k++){
        board[k]='-';
    }

    //makes arrays of pointers corrsponding to rows, columns and diagonals of the board 
    p=board;    //pointer initialization 
    row1[0]=p;row1[1]=p+1;row1[2]=p+2; //starting from here the pointers arrays corrspond to rows1,2,3:cols 1,2,3 nad diagonals 1 & 2 respectively
    row2[0]=p+3;row2[1]=p+4;row2[2]=p+5;
    row3[0]=p+6;row3[1]=p+7;row3[2]=p+8;
    col1[0]=p;col1[1]=p+3;col1[2]=p+6;
    col2[0]=p+1;col2[1]=p+4;col2[2]=p+7;
    col3[0]=p+2;col3[1]=p+5;col3[2]=p+8;
    diag1[0]=p;diag1[1]=p+4;diag1[2]=p+8;
    diag2[0]=p+2;diag2[1]=p+4;diag2[2]=p+6;
}


bool game::is_the_move_valid(int move){
    if(board[move]=='-'){
        return true;
    }
    if(move<9 && board[move]!='-'){
        SetColor(RED);
        std::cout<<"That Position is alraedy taken!!!\n\n";
        SetColor(WHITE);
    }

    if(move>9){
        SetColor(RED);
        std::cout<<"That is an illegal choice!!!\n\n";
        SetColor(WHITE);
    }

    return false;
}

bool game::are_elements_equal(std::array<char*,3> k){
    std::array<char*,3>::iterator p;
    p=k.begin();
    if(**p==**(p+1) && **(p+1)==**(p+2)){
        return true;
    }

    else {
        return false;
    }
}

state game::state_of_game(){
    if(   (are_elements_equal(row1)==true && *row1[0]!='-')
        ||(are_elements_equal(row2)==true && *row2[0]!='-')
        ||(are_elements_equal(row3)==true && *row3[0]!='-')
        ||(are_elements_equal(col1)==true && *col1[0]!='-')
        ||(are_elements_equal(col2)==true && *col2[0]!='-')
        ||(are_elements_equal(col3)==true && *col3[0]!='-')
        ||(are_elements_equal(diag1)==true && *diag1[0]!='-')
        ||(are_elements_equal(diag2)==true && *diag2[0]!='-')){

            return WIN;
    }

    else if(board_full()==true){
        return DRAW;
    }

    else{
    return KEEP_PLAYING;
    }

}

bool game::board_full(){
    for(int i=0;i<9;i++){
            if(board[i]=='-'){
                return false;
            }
        }
    return true;
}
void game::display_board(){
    std::cout<<" "<<board[0]<<"  | "<<board[1]<<"  | "<<board[2]<<"  \n____|____|____\n "<<board[3]<<"  | "<<board[4]<<"  | "<<board[5]<<"  \n____|____|____\n "<<board[6]<<"  | "<<board[7]<<"  | "<<board[8]<<"  \n    |    |    \n\n";
}

void game::win_display(){
    if(are_elements_equal(row1)==true){
        std::cout<<" ";
        color_conv(board[0]);
        std::cout<<"  | ";
        color_conv(board[1]);
        std::cout<<"  | ";
        color_conv(board[2]);
        std::cout<<"  \n____|____|____\n "<<board[3]<<"  | "<<board[4]<<"  | "<<board[5]<<"  \n____|____|____\n "<<board[6]<<"  | "<<board[7]<<"  | "<<board[8]<<"  \n    |    |    \n\n";
        Winner(*row1[0]);
    }

    else if(are_elements_equal(row2)==true){
        std::cout<<" "<<board[0]<<"  | "<<board[1]<<"  | "<<board[2]<<"  \n____|____|____\n ";
        color_conv(board[3]);
        std::cout<<"  | ";
        color_conv(board[4]);
        std::cout<<"  | ";
        color_conv(board[5]);
        std::cout<<"  \n____|____|____\n "<<board[6]<<"  | "<<board[7]<<"  | "<<board[8]<<"  \n    |    |    \n\n";
        Winner(*row2[0]);
    }

    else if(are_elements_equal(row3)==true){
        std::cout<<" "<<board[0]<<"  | "<<board[1]<<"  | "<<board[2]<<"  \n____|____|____\n "<<board[3]<<"  | "<<board[4]<<"  | "<<board[5]<<"  \n____|____|____\n ";
        color_conv(board[6]);
        std::cout<<"  | ";
        color_conv(board[7]);
        std::cout<<"  | ";
        color_conv(board[8]);
        std::cout<<"  \n    |    |    \n\n";
        Winner(*row3[0]);
    }

    else if(are_elements_equal(col1)==true){
        std::cout<<" ";
        color_conv(board[0]);
        std::cout<<"  | "<<board[1]<<"  | "<<board[2]<<"  \n____|____|____\n ";
        color_conv(board[3]);
        std::cout<<"  | "<<board[4]<<"  | "<<board[5]<<"  \n____|____|____\n ";
        color_conv(board[6]);
        std::cout<<"  | "<<board[7]<<"  | "<<board[8]<<"  \n    |    |    \n\n";
        Winner(*col1[0]);
    }

    else if(are_elements_equal(col2)==true){
        std::cout<<" "<<board[0]<<"  | ";
        color_conv(board[1]);
        std::cout<<"  | "<<board[2]<<"  \n____|____|____\n "<<board[3]<<"  | ";
        color_conv(board[4]);
        std::cout<<"  | "<<board[5]<<"  \n____|____|____\n "<<board[6]<<"  | ";
        color_conv(board[7]);
        std::cout<<"  | "<<board[8]<<"  \n    |    |    \n\n";
        Winner(*col2[0]);
    }

    else if(are_elements_equal(col3)==true){
        std::cout<<" "<<board[0]<<"  | "<<board[1]<<"  | ";
        color_conv(board[2]);
        std::cout<<"  \n____|____|____\n "<<board[3]<<"  | "<<board[4]<<"  | ";
        color_conv(board[5]);
        std::cout<<"  \n____|____|____\n "<<board[6]<<"  | "<<board[7]<<"  | ";
        color_conv(board[8]);
        std::cout<<"  \n    |    |    \n\n";
        Winner(*col3[0]);
    }

    else if(are_elements_equal(diag1)==true){
        std::cout<<" ";
        color_conv(board[0]);
        std::cout<<"  | "<<board[1]<<"  | "<<board[2]<<"  \n____|____|____\n "<<board[3]<<"  | ";
        color_conv(board[4]);
        std::cout<<"  | "<<board[5]<<"  \n____|____|____\n "<<board[6]<<"  | "<<board[7]<<"  | ";
        color_conv(board[8]);
        std::cout<<"  \n    |    |    \n\n";
        Winner(*diag1[0]);
    }

    else if(are_elements_equal(diag2)==true){
        std::cout<<" "<<board[0]<<"  | "<<board[1]<<"  | ";
        color_conv(board[2]);
        std::cout<<"  \n____|____|____\n "<<board[3]<<"  | ";
        color_conv(board[4]);
        std::cout<<"  | "<<board[5]<<"  \n____|____|____\n ";
        color_conv(board[6]);
        std::cout<<"  | "<<board[7]<<"  | "<<board[8]<<"  \n    |    |    \n\n";
        Winner(*diag2[0]);
    }

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

The human class has one function, play(), which makes sure the players play in a turn-by-turn fashion.

This is implemented by odd/even checks of the integer i. If a player makes an invalid move, a warning is given and the player gets another chance.

human.h

#ifndef __HUMAN_H_INCLUDE
#define __HUMAN_H_INCLUDE

#include "game.h"
#include <sstream>
class human :
    public game
{
public:
    void play();
};

#endif

human.cpp

#include "stdafx.h"
#include "human.h"
#include <sstream>

void human::play(){
    int i=1,player_choice;
    char mark;
    std::string current_palyer;
    SetColor(DARKYELLOW);
    std::cout<<"Player 1:Enter your Name\n";
    getline(std::cin, dummy);// weird problem with getline if not for this line it won't accept player1's name 
    getline(std::cin, player1);
    std::cout<<"Player 2:Enter your Name\n";
    getline(std::cin, player2);
    SetColor(WHITE);

    while(state_of_game()==KEEP_PLAYING){
        if(i%2!=0){
            current_palyer=player1;
            mark='O';
        }
        else if(i%2==0){
            current_palyer=player2;
            mark='X';
        }
        std::cout<<current_palyer<<":Enter your Choice....\n";
        std::cin>>player_choice;

        if(is_the_move_valid(player_choice-1)==true){
            board[player_choice-1]=mark;
            display_board();
        }
        else {
            i--;
        }
        i++;
    }

    if(state_of_game()==DRAW){
        SetColor(PINK);
        std::cout<<"The Game is a Draw!!!\n";
        SetColor(WHITE);
    }

    else if(state_of_game()==WIN){
        win_display();
    }

}

This class implements AI for the computer player and the AI rules. As described by @shade:

  • if there is a possibility of a win, take it
  • if there is a block, take it
  • if the center is available, take it
  • if any of the the corners are available, take it randomly
  • if any of the middle edges (i.e positions 1,3,5,7) are available, take it randomly

computer.h

#ifndef __COMPUTER_H_INCLUDE
#define __COMPUTER_H_INCLUDE
#include "game.h"
#include <vector>
#include <time.h>

enum specifier {CORNERS=0, MIDDLE_CORNERS};
enum status {WINS,BLOCK,NONE};

class computer :
    public game
{
protected:
    struct win_block_info{
        bool wins;
        bool blocks;
        int place;
    };
    struct corner_info{
        bool corners;
        bool mid_corners;
        std::vector<char*> empty_pos;
    };
    std::vector<std::array<char*,3>> list;
    std::vector<status> status_list;
    std::array<char*,4> corners;
    std::array<char*,4> middle_corners;
public:
    computer();
    void play();
    void comp_choice();
    win_block_info possible_wins_or_blocks();
    corner_info corner_placement();
    bool any_two_elements_equal(std::array<char*,3>);
    void test();
    std::vector<char*> empty_places_list(specifier);
    ~computer();

};
#endif

computer.cpp

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


computer::computer(){
    srand((time(NULL)));
    list.push_back(row1);
    list.push_back(row2);
    list.push_back(row3);
    list.push_back(col1);
    list.push_back(col2);
    list.push_back(col3);
    list.push_back(diag1);
    list.push_back(diag2);
    corners[0]=p;corners[1]=p+2;corners[2]=p+6;corners[3]=p+8;
    middle_corners[0]=p+1;middle_corners[1]=p+3;middle_corners[2]=p+5;middle_corners[3]=p+7;
}

void computer::play(){
    int i=1,player_choice;
    char mark;
    SetColor(DARKYELLOW);
    std::cout<<"Player: Enter your Name\n";
    getline(std::cin, dummy);// weird problem with getline if not for this line it won't accept player1's name 
    getline(std::cin, player1);
    player2="Computer";
    SetColor(WHITE);

    while(state_of_game()==KEEP_PLAYING){
        if(i%2!=0){
            mark='O';
            std::cout<<player1<<":Enter your Choice....\n";
            std::cin>>player_choice;
            if(is_the_move_valid(player_choice-1)==true){
                board[player_choice-1]=mark;
                display_board();
            }
            else {
                i--;
            }
        }

        else if(i%2==0){
            std::cout<<player2<<":has made the Choice....\n";
            comp_choice();
            display_board();
        }
        i++;
    }

    if(state_of_game()==DRAW){
        SetColor(PINK);
        std::cout<<"The Game is a Draw!!!\n";
        SetColor(WHITE);
    }

    else if(state_of_game()==WIN){
        win_display();
    }
}

void computer::comp_choice(){
    win_block_info status;
    corner_info corner_status;
    status=possible_wins_or_blocks();
    if(status.wins==true){
        for(int k=0;k<3;k++){
            if(*list[status.place][k]=='-'){
                *list[status.place][k]='X';
                list.erase(list.begin()+status.place);
                return;
            }
        }
    }

    if(status.blocks==true){
        for(int k=0;k<3;k++){
            if(*list[status.place][k]=='-'){
                *list[status.place][k]='X';
                list.erase(list.begin()+status.place);
                return;
            }
        }
    }

    if(board[4]=='-'){
        board[4]='X';
        return;
    }

    corner_status=corner_placement();
    if(corner_status.corners==true){
        int ran=std::rand()%corner_status.empty_pos.size();
        *corner_status.empty_pos[ran]='X';
        corner_status.empty_pos.clear();
        return;
    }

    if(corner_status.mid_corners==true){
        int ran=std::rand() % corner_status.empty_pos.size();
        *corner_status.empty_pos[ran]='X';
        corner_status.empty_pos.clear();
        return;
    }
}

computer::win_block_info computer::possible_wins_or_blocks(){
    win_block_info status;
    status.blocks=false;
    status.wins=false;
    status.place=10000;
    for(int i=0;i<list.size();i++){
        if(any_two_elements_equal(list[i])==true){
            status.blocks=true;
            status.place=i;
            if(*list[i][0]=='X' || *list[i][1]=='X'){
            status.wins=true;
            status.blocks=false;
            status.place=i;
            return status;
            }
        }
    }

    return status;
}



bool computer::any_two_elements_equal(std::array<char*,3> current){

    if(   (*current[0]==*current[1] && *current[0]!='-')
        ||(*current[1]==*current[2] && *current[1]!='-')
        ||(*current[2]==*current[0] && *current[2]!='-')){
            if(*current[0]=='-'||*current[1]=='-'||*current[2]=='-'){
        return true;
        }
    }

    return false;
}

computer::corner_info computer::corner_placement(){
    corner_info status;
    status.corners=false;
    status.mid_corners=false;
    for(int i=0;i<4;i++){
        if(*corners[i]=='-'){
            status.corners=true;
            status.empty_pos.push_back(corners[i]);
        }
    }
    if(status.corners==true){
        return status;
    }

    for(int i=0;i<4;i++){
        if(*middle_corners[i]=='-'){
            status.mid_corners=true;
            status.empty_pos.push_back(corners[i]);
        }
    }

    return status;
}

void computer::test(){
    std::cout<<possible_wins_or_blocks().wins<<" "<<possible_wins_or_blocks().place<<std::endl;
    comp_choice();
    display_board();
}

computer::~computer(){
    list.clear();
}
\$\endgroup\$
7
\$\begingroup\$

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 << board;

            // Get Human input
            while(!(std::cin >> result))
            {
                // Bad input.
                // Fix stream.
            }

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


class Game
{
     public:
        Game(std::unique_ptr<Player> p1, std::unique_ptr<Player> 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 <iostream>
#include <string>
#include <Windows.h>
#include "game.h"
#include "human.h"
#include "computer.h"
#include <sstream>

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 <string>        // 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 <unistd.h>

// Now System header files
#include <ICantThinkOfAnything>

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 <iostream>
#include <sstream>
#include <string>
// C Header
// System Headers
#include <Windows.h>

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 ownership so pointer is OK.
};
Win operator<<(std::ostream& s, Win w)
{
    return Win(w, s);   // Save the stream, as we are returning a Win object
}
template<typename T>
std::ostream& operator<<(Win const& w, T const& data)
          //             ^^^^^^^^^^^^    Notice the first parameter is not a stream
{
    std::ostream& s = *(w.sptr);  // Retrieve the saved stream (see above)
    if (w.good) {SetColor(TEAL);} 
    s << data;
    if (w.good) {SetColor(WHITE);}
    return s;
}

// SO
std::cout << Win(true) << "X" << "Y";
// ^^^^^^^^^^^^^^^^^^^^^^   Returns a "Win" object
//                          See the first operator<< above.

// This is equivalent to:
operator<<(operator<<(operator<<(std::cout, Win(true)), "X"), "Y");

// If we break this into three lines we get
Win            tmp1 = operator<<(std::cout, Win(true));
// because we return a Win from `operator<<` when the second argument is a win!

// This takes a Win as the first argument and will return the original stream.
// But this allows us to colour any value conditionally on a win.
std::ostream&  tmp2 = operator<<(tmp1, "X");
std::ostream&  tmp3 = operator<<(tmp2, "Y");
\$\endgroup\$
  • 1
    \$\begingroup\$ How about Player &winner = (c == 'O' ? player1 : player2);? \$\endgroup\$ – idoby Aug 26 '13 at 13:41
  • \$\begingroup\$ the stream manipulators is totally new to me..gotta read up on that one ....so far i understood till the template i'll try and read as much as i can and comeback with questions....meanwhile can you point what i need to learn to understand these concepts? thank you \$\endgroup\$ – Anirudha Adibhatla Aug 26 '13 at 21:15
  • 2
    \$\begingroup\$ @AnirudhaAdibhatla: Just remember that operator<< is a normal function with a funny name. That X << Y is short hand for: operator<<(X,Y). \$\endgroup\$ – Martin York Aug 26 '13 at 21:52
  • \$\begingroup\$ hi can you explain what ownership of pointers is? I tried googling it but it's all kind of confusing.... \$\endgroup\$ – Anirudha Adibhatla Aug 27 '13 at 1:24
  • \$\begingroup\$ hi like i mentioned i'm totally new to the everything mentioned in the stream manipulator part ....but i think i got the hang of it ...i'll explain what i understood so far step-by-step and correct where i'm wrong, also may be fill in the left out parts.... \$\endgroup\$ – Anirudha Adibhatla Aug 27 '13 at 2:16
6
\$\begingroup\$
  • In general: it's nice to organize your STL #includes alphabetically. In addition, the header #includes should be above the library ones so that there are no dependencies among them.

  • Not a very effective choice as you're allowing any value below 4. You also need a default for when an invalid choice is entered. For a more readable main loop, put the switch in a for(;;). This essentially runs the game "forever" until the user quits. You also don't need the replay since choice serves the same purpose.

    Here's roughly one suggestion:

    void main_play()
    {
        for (;;)
        {
            std::cout << "/* displayed stuff */";
            int choice; // move this right next to the cin
            std::cin >> choice;
    
            switch (choice)
            {
            case 1:
                // start game
                break;
            case 2:
                // start game
                break;
            case 3:
                // quit game
                return;
            default:
                // if the user enters an invalid value,
                // throw an exception and allow the loop
                // to restart safely for the next input
                // #include <stdexcept> to use this
    
                throw std::logic_error("Invalid Choice");
            }
        }
    }
    
  • I'd put all the enums into either the appropriate class or a namespace. Having them in global scope can cause name clashes and disorganization.

  • You're doing a bit much to set up the board. Just make a "2D" std::array:

    std::array<std::array<char, 3>, 3> board; // one array in the other
    

    To ease the readability when using it, make a typedef:

    typedef std::array<std::array<char, 3>, 3> Board;
    
  • You don't use <sstream> at all in human.h, so leave it out. Also, I think you've meant to use <string> instead of <sstream> in human.cpp.

  • I'd put ~computer() below computer() in the header and implementation.

  • In computer.h: use <ctime> instead of <time.h>. The latter is a C library.

  • This one should stand out. In computer.cpp: move the srand() to main() and take out any other calls to this.

    Putting this here:

    1. makes it easy to keep track of the initial seeding
    2. prevents the "same randomness" from occurring at each call to rand()

    After you do this, make these other changes:

    #include <ctime>   // move this from computer.h
    #include <cstdlib> // for srand()
    
    // this goes at the top of main()
    // change your current srand() to this
    // if you're using C++11, use nullptr
    std::srand(static_cast<unsigned int>(std::time(NULL)));
    

    If you are using C++11, srand/rand should not be used at all as it is considered harmful. Instead, the <random> library should be utilized.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.