3

I need to make a copy of GameMaster* thisMaster so I can preform manipulations while still maintaining a "clean" copy. However, the way I'm doing it right now, when I make a change to copy, it changes thisMaster too.

void Move::possibleMoves(GameMaster* thisMaster)
{
     GameMaster* copy = new GameMaster(*thisMaster);
}

How can I fix this?

edit: I created a copy constructor but am still having the same issue.

GameMaster::GameMaster(const GameMaster& gm)
{
    for(int i=0;i<GAMETILES;i++)
    {
        gameTiles[i]=gm.gameTiles[i];
    }
    players=gm.players;
    vertLines=gm.vertLines;
    horLines=gm.horLines;
    turn = gm.turn;
    masterBoard=gm.masterBoard;
    lastLegal=gm.lastLegal;
    lastScore=gm.lastScore;
}

Here is the complete class definition for GameMaster:

Class GameMaster
{
public:
    GameMaster(void);
    GameMaster(const GameMaster& gm);
    ~GameMaster(void);
    //functions

private:
    std::vector <Player*> players;
    std::vector <Line> vertLines;
    std::vector <Line> horLines;
    Tile gameTiles [GAMETILES];
    std::vector <std::string>colors;
    std::vector <std::string>shapes;
    int turn;
    Board masterBoard;
    bool lastLegal;
    int lastScore;
};

With the copy constructor, I am still having the issue with the Board changing values. Does it need a copy constructor too?

  • 5
    Chances are your copy constructor is shallow-copying a member pointer that you use to make your comparison. As a side note, is that local pointer necessary? Why not `GameMaster copy (*thisMaster);`? – chris Aug 24 '12 at 05:48
  • 1
    It would be very helpful to post the declaration of your GameMaster class, and the implementation of your copy constructor. – razlebe Aug 24 '12 at 05:50
  • @AndrésSenac, Not unless it's implemented correctly. Anyway, why bother writing one? Adhere to the [Rule of Zero](http://rmartinho.github.com/2012/08/15/rule-of-zero.html) and you'll save yourself some code and possible memory issues such as this one. – chris Aug 24 '12 at 05:51
  • @chris Sorry, I'm a bit new to C++. I didn't know I had to make a copy constructor, so I guess I'm using the default one. Do I need a copy constructor? Also, I made a new local one because I was having a stack overflow before. –  Aug 24 '12 at 05:56
  • @Kyryx, Stack overflow? Either you have some other problem, or that is one big set of classes. Judging from the function name, though, if this is an AI calling `possibleMoves` a lot in order to decide where to go, that would make sense. For information on the why (and how), see this question on the [Rule of Three](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). In the long run, the rule of zero has better results, though. – chris Aug 24 '12 at 06:00
  • @chris Yeah, unfortunately it is just a big set of classes. I [posted a question about it earlier](http://stackoverflow.com/questions/12102242/c-stack-overflow-in-nested-function#comment16176209_12102242) and that seemed to be the limited consensus. The rule of three is very interesting. I am reading it now. Thank you for the help –  Aug 24 '12 at 06:05
  • Yes, if you are making copies and have non-trivial data in the class (e.g. pointers or collection of pointers) you need to make your own copy constructor. – Some programmer dude Aug 24 '12 at 06:08
  • @JoachimPileborg I just edited to include the copy constructor. I'm still having the same issue. Does it look okay? Do the objects in it (eg. masterBoard) need copy constructors too? –  Aug 24 '12 at 06:22
  • looks ok to me.. where are you using copy-> ? and where do you declare gameTiles, players, etc.. are they pointer by any chance? – jaybny Aug 24 '12 at 06:38
  • @jaybny I am using it in a completely separate class. Should I include the method that uses it? The one I am for sure having an issue with is `Board`. It is an array of `Tile*`. Do I need to put a copy constructor in `Board` too? Edit: or do I want a copy assignment operator? –  Aug 24 '12 at 06:41
  • you need to post all your code.. you say that Board is a **Tile? Is it a typdef? just post your code and describe what your trying to do and what your bug is. – jaybny Aug 24 '12 at 06:47

2 Answers2

2
Class GameMaster
{
public:
    GameMaster(void);
    GameMaster(const GameMaster& gm);
    ~GameMaster(void);
    //functions

private:
    std::vector <Player*> players; // You should make a deep copy because of pointers
    std::vector <Line> vertLines; // Shallow copy is OK if Line doesn't have pointers in it
    std::vector <Line> horLines; // see above
    Tile gameTiles [GAMETILES]; // One by one assignment is OK
    std::vector <std::string>colors; // Shallow copy is OK
    std::vector <std::string>shapes; // Shallow copy is OK
    int turn; // assignment is OK
    Board masterBoard; // same questions for GameMaster still exists for copying this
    bool lastLegal; // assignment is OK
    int lastScore; // assignment is OK
};

Here is link for Shallow vs Deep copy

Seçkin Savaşçı
  • 3,446
  • 2
  • 23
  • 39
  • Thanks for the help, reading up on deep copy now. I am assuming for Board I want to give it a copy assignment operator, because I am using `=` right? –  Aug 24 '12 at 06:50
  • Not exactly, the problem of masterBoard is if it contains vectors of pointers or pointers like in the situation of GameMaster class then you should write the copy constructor for it, too. I assume you want a deep copy of the masterBoard class, if it's not the situation you can simply shallow copy it without implementing its copy constructor. – Seçkin Savaşçı Aug 24 '12 at 06:52
0

players=gm.players;

is only copying the collection of pointers.. you will need to do a deep copy of the players in the new vector.

edit

for( auto iter = gm.players.begin(); iter != gm.players.end(); ++iter) 
{
   players.push_back(new Players(*iter))
}

you will also need to have a copy const for Players.

cheers

jaybny
  • 1,026
  • 2
  • 13
  • 29
  • I thought it might be something like that. I'm sorry, I'm not familiar with what a deep copy is. How do I do one? –  Aug 24 '12 at 06:45