0

I have had a similar issue with quite a few projects I have worked on involving classes containing arrays. I have a class that is supposed to handle a 2 dimensional matrix representing a TicTacToe game. There is an enumeration in the class for the status of the current game and one member function that has an enumeration return type. I cant seem to figure out why I can create the class set values in the matrix and as soon as I call the member function with the enumerated return type the whole array is reinitialized to 0. I think it has something to do with the constructor being called again or something along those lines but I have not been able to find anything after searching for the past few hours. Any help would be greatly appreciated.

Here is my header file containing the class information:

#ifndef TTT_H
#define TTT_H

#include <cstdlib>
#include <iostream>

using namespace std;

class TicTacToe
{
private:
    enum Status{WinX, WinO, Continue, Draw};
    int **board;

public:
    TicTacToe();
    ~TicTacToe();
    void PrintBoard();
    bool ValidMove(int, int);
    bool PlayerMove(int, int, int);
    Status GameStatus();    //this one causes the problem
    void Debug();
};

#endif 

Here is the code for CPP file with the member function definitions:

#include "TicTacToe.h"
#include <iostream>
#include <cstdlib>
#include <iomanip>
#include <cassert>

using namespace std;

TicTacToe::TicTacToe() 
{
    board = new int*[3];
    assert(board != 0);

    for(int i=0;i<3;i++)
    {
        cout << "Constructor Ran again" << endl;    //for testing
        board[i] = new int[3];
        assert(board[i] != 0);
        for(int j=0;j<3;j++)
            board[i][j] = 9;
    }
}

TicTacToe::TicTacToe(TicTacToe &copy)
{
    board = new int*[3];
    assert(board != 0);
}

TicTacToe::~TicTacToe()
{
    if(board)
        delete[] board;
}

void TicTacToe::PrintBoard() 
{
    for(int i=0;i<3;++i)
    {
        for(int j=0;j<3;++j)
        {
            cout << "| ";

            switch(board[i][j]){
            case 0:
                cout << "O ";
                break;
            case 1:
                cout << "X ";
                break;
            case 9:
                cout << "  ";
                break;
            }
        }
        cout << "|" << endl;
        cout << "------------" << endl;
    }
}

bool TicTacToe::ValidMove(int row, int col) 
{
    bool valid = false;
    if(row < 3 && col < 3)
    {
        if(board[row][col] == 9)
            valid = true;
    }
    return valid;
}

bool TicTacToe::PlayerMove(int player, int row, int col) 
{
    bool done = false;

    if(ValidMove(row,col) == true)
    {
        if(player == 1)
            board[row][col] = 1;
        else
            board[row][col] = 0;
        done = true;
    }

    return done;
}

TicTacToe::Status TicTacToe::GameStatus() //This function is the problem
{
    int check, empty = 0;
    bool done = false;

    for(int i=0;i<3;++i)
    {
        for(int j=0;j<3;++j)
        {
            check += board[i][j];
            if(board[i][j] = 9)
                empty++;
        }
        if(check == 0)
            return WinO;
        else if(check == 3)
            return WinX;
        check = 0;
    }

    if(empty == 0)
        return Draw;

    for(int i=0;i<3;++i)
    {
        for(int j=0;j<3;++j)
            check += board[j][i];
        if(check == 0)
            return WinO;
        else if(check == 3)
            return WinX;
        check = 0;
    }

    check = board[0][0] + board[1][1] + board[2][2];
    if(check == 0)
        return WinO;
    else if(check == 3)
        return WinX;
    check = 0;

    check = board[0][2] + board[1][1] + board[2][0];
    if(check == 0)
        return WinO;
    else if(check == 3)
        return WinX;
    check = 0;


    return Continue;
}



void TicTacToe::Debug()
{
    //cout << &board[0][0] << endl;
    for(int i=0;i<3;++i)
    {
        for(int j=0;j<3;++j)
            cout << board[i][j];
        cout << endl;
    }
}

Here is the driver file I am using to test:

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

using namespace std;

int main()
{
    int row, col;
    bool valid;
    enum Status{WinX, WinO, Continue, Draw};
    TicTacToe * T;
    T = new TicTacToe;
    assert(T != 0);

    cout << "There are 2 players. P1 is x P2 is o" << endl;

    do
    {
        T->PrintBoard();

        valid = false;
        while(valid == false)
        {
            cout << "\nP1 choose a cell" ;
            cin >> row >> col;

            if(T->ValidMove(row, col) == true)
            {
                T->PlayerMove(1, row, col);
                valid = true;
            }
            else
            {
                cout << "Not a valid choice" << endl;
                valid = false;  
            }

        }

        T->PrintBoard();
        cout << endl;

        T->GameStatus();  //<<<<<this is the pain in my butt

        T->PrintBoard();

        valid = false;
        while(valid == false)
        {
            cout << "\nP2 choose a cell" ;
            cin >> row >> col;

            if(T->ValidMove(row, col) == true)
            {
                T->PlayerMove(2, row, col);
                valid = true;
            }
            else
            {
                cout << "Not a valid choice" << endl;
                valid = false;  
            }
        }   
    }
    while(/*T->GameStatus() == Continue*/ 1==1);
                  //the call to GameStatus was commented out of the
                  //while statement for testing


    return 0;

    }

I know the code inside of the GameStatus function is far from pretty but the array is messed up before any of those lines are processed.

I left all of the other functions just to show that they work properly without issue.

Thanks in advance for any help you may be able to give.

bvallerand
  • 49
  • 1
  • 10

4 Answers4

3

You've got a simple typo in your code..

if(board[i][j] = 9) // will always return true (refp)
  empty++;

Other remarks

When looking at your code a bit more thoroughly I see that you have a few other miss-happens, intentional or unintentional.. that I don't know:

  • int check is not initialized in TicTacToe::GameStatus
  • You are not freeing the allocated memory properly, you'll need to free all entries in board, ie. delete board[i])

I don't like bugs, how can I get rid of the operator= vs operator== problem?

A quite common method to circumvent the problem of making a typo and writing = when you mean to compare (==) two variables is to flip the operands around (if one of them is a constant value, such as 9.

if(9 = board[i][j]) will not compile and such a bug would've never appeared in your code.

I'll have to say that I don't like writing my statements that way.. though it's a quite common method, especially in the "beginner" segment.

Community
  • 1
  • 1
Filip Roséen - refp
  • 62,493
  • 20
  • 150
  • 196
  • Another quite common method to get rid of that problem is to enable your compiler warnings. :) – Kos Dec 20 '11 at 11:52
1

check is not initialized in GameStatus() .

Henrik
  • 23,186
  • 6
  • 42
  • 92
1
if (board[i][j] = 9)

Isn't the above line of code resetting the array contents? You probably want to use == here, instead.

Cody Gray - on strike
  • 239,200
  • 50
  • 490
  • 574
1

You have a serious issue in memory management. Look:

  • Your constructor performs 4 allocations (an array of pointers and 3 arrays of ints, to emulate a 2D arrray),
  • Your destructor performs 1 deallocation (= memory leak),
  • You have a custom destructor but you don't define (or block) operator= (you need to, see What is The Rule of Three?)
  • Your copy constructor is incomplete and doesn't create a "valid" object.

Basically the above is likely to cause you some memory problems. I suggest to:

  • Rewrite the destructor to first free all the arrays of ints, then the array of pointers,
  • Make the class TicTacToe uncopiable by declaring the copy constructor and the operator= as private.

Also some minor details on that matter:

board = new int*[3];
assert(board != 0);

The assertion is unnecessary. If the allocation fails, the operator new will throw an exception.

if(board)
    delete[] board;

Operators delete and delete[] don't do anything if their argument is a null pointer, so that condition is redundant. Also you have designed your object with the invariant that the board exists as long as the TicTacToe object exists, so that check is totally unnecessary, right?

Keep it safe and simple!

Community
  • 1
  • 1
Kos
  • 70,399
  • 25
  • 169
  • 233
  • +1. Maybe also add that his copy constructor is not only incomplete, but has the wrong signature. Either should take `const T&`, or `T`. – Sebastian Mach Dec 20 '11 at 11:53
  • `T&` is allowed but `const T&` is the "correct" signature, good catch! T is invalid, however. – Kos Dec 20 '11 at 11:59
  • Thank you everybody for all of your help. fixing the =9 issue fixed my data issue and hows this for a destructer: TicTacToe::~TicTacToe() { if(board) { for(int i=2;i>=0;--i) { delete[] board[i]; } delete[] board; } } – bvallerand Dec 20 '11 at 13:49
  • Wow sorry that did not post the way I wanted it to. – bvallerand Dec 20 '11 at 13:51