// Game Lobby 2.0 Exersize Enhancements
// Simulates a game lobby where players wait
#include <iostream>
#include <string>
using std::cout;
using std::cin;
using std::endl;
using std::string;
using std::ostream;
class Player
{
public:
Player(const string& name = "") : m_Name(name), m_pNext(0) {}
string GetName() const {return m_Name;}
Player* GetNext() const {return m_pNext;}
void SetNext(Player* next) {m_pNext = next;}
private:
string m_Name;
Player* m_pNext; // pointer to the next player in list
};
class Lobby
{
friend ostream& operator<<(ostream& os, const Lobby& aLobby);
public:
Lobby(): m_pHead(0) {}
~Lobby() {Clear();}
void AddPlayer();
void RemovePlayer();
void Clear();
Player GetPlayer() const {return *m_pHead;}
Lobby(const Lobby& aLobby) // Copy Constructor
{
cout << "Copy Constructor Called...\n\n";
m_pHead = 0;
*m_pHead = aLobby.GetPlayer();
}
private:
Player* m_pHead;
};
void Lobby::AddPlayer()
{
// create a new player node
cout << "Please enter the name of the new player: ";
string name;
cin >> name;
Player* pNewPlayer = new Player(name);
// if list is empty, make head of list this new player
if (m_pHead == 0)
{
m_pHead = pNewPlayer;
}
// otherwise find the end of the list and add the player there
else
{
Player* pIter = m_pHead;
while (pIter->GetNext() != 0)
{
pIter = pIter->GetNext();
}
pIter->SetNext(pNewPlayer);
}
}
void Lobby::RemovePlayer()
{
if (m_pHead == 0)
{
cout << "The game lobby is empty. No one to remove!\n";
}
else
{
Player* pTemp = m_pHead;
m_pHead = m_pHead->GetNext();
delete pTemp;
}
}
void Lobby::Clear()
{
while (m_pHead != 0)
{
RemovePlayer();
}
}
ostream& operator<<(ostream& os, const Lobby& aLobby)
{
Player* pIter = aLobby.m_pHead;
os << "\nHere's who's in the game lobby:\n";
if (pIter == 0)
{
os << "The lobby is empty.\n";
}
else
{
while (pIter != 0)
{
os << pIter->GetName() << endl;
pIter = pIter->GetNext();
}
}
return os;
}
void testCopy(Lobby aLobby);
int main()
{
Lobby mLobby;
int choice;
do {
cout << mLobby;
cout << "\nGame Lobby\n";
cout << "0 - Exit the program.\n";
cout << "1 - Add a player to the lobby\n";
cout << "2 - Remove a player from the lobby.\n";
cout << "3 - Clear the lobby.\n";
cout << "4 - Copy the Lobby.\n";
cout << endl << "Enter choice: ";
cin >> choice;
switch(choice)
{
case 0: cout << "Good-Bye\n"; break;
case 1: mLobby.AddPlayer(); break;
case 2: mLobby.RemovePlayer(); break;
case 3: mLobby.Clear(); break;
case 4: testCopy(mLobby); break;
default: cout << "That was not a valid choice.\n";
}
} while(choice != 0);
return 0;
}
void testCopy(Lobby aLobby)
{
cout << "Copy Complete?\n\n";
}
Help Copy Constructor
Author
Hello everyone,
I am reading "Beginning C++ Game Programming" and I am stuck with Exersize 1 in chapter 9. I am trying to write a copy constructor and it blows up. What am I doing wrong here?
Your copy contains the same pointer as the original; you're not copying the player objects.
When the copy gets destroyed, it destroys the Players, and then the original's pointer will be pointing to garbage, as the Players aren't there anymore.
When the copy gets destroyed, it destroys the Players, and then the original's pointer will be pointing to garbage, as the Players aren't there anymore.
Author
Thanks for the quick reply smart_idiot.
How would I properly write the copy constructor?
How would I properly write the copy constructor?
Lobby(const Lobby& aLobby) // Copy Constructor
{
cout << "Copy Constructor Called...\n\n";
m_pHead = 0;
*m_pHead = aLobby.GetPlayer();
}
private:
Player* m_pHead;
-------------------------------------------------------------------
Copy Constructor looks good but copy assignment is missing. So the compiler generates one for you. Like this:-
Lobby & operator=(const Lobby& lobby)
{
m_pHead = lobby.m_pHead;
}
I hope you know the side effect of this (Memory address of lobby.m_pHead is copied, not the contents in the memory address lobby.m_pHead). I think you should be fine if you provide a copy assignment function that copies the contents but not the address.
{
cout << "Copy Constructor Called...\n\n";
m_pHead = 0;
*m_pHead = aLobby.GetPlayer();
}
private:
Player* m_pHead;
-------------------------------------------------------------------
Copy Constructor looks good but copy assignment is missing. So the compiler generates one for you. Like this:-
Lobby & operator=(const Lobby& lobby)
{
m_pHead = lobby.m_pHead;
}
I hope you know the side effect of this (Memory address of lobby.m_pHead is copied, not the contents in the memory address lobby.m_pHead). I think you should be fine if you provide a copy assignment function that copies the contents but not the address.
Author
I have written a new assignment operator but I still blowup on *m_pHead = aLobby.GetPlayer(); inside the copy constructor.
here is the new assignment operator:
Here is the updated source:
here is the new assignment operator:
Lobby& operator=(const Lobby& aLobby) { if (this == &aLobby) { return *this; } else { *m_pHead = aLobby.GetPlayer(); return *this; } }Here is the updated source:
// Game Lobby 2.0 Exersize Enhancements// Simulates a game lobby where players wait#include <iostream>#include <string>using std::cout;using std::cin;using std::endl;using std::string;using std::ostream;class Player{public: Player(const string& name = "") : m_Name(name), m_pNext(0) {} string GetName() const {return m_Name;} Player* GetNext() const {return m_pNext;} void SetNext(Player* next) {m_pNext = next;}private: string m_Name; Player* m_pNext; // pointer to the next player in list};class Lobby{ friend ostream& operator<<(ostream& os, const Lobby& aLobby);public: Lobby(): m_pHead(0) {} ~Lobby() {Clear();} void AddPlayer(); void RemovePlayer(); void Clear(); Player GetPlayer() const {return *m_pHead;} Lobby& operator=(const Lobby& aLobby) { if (this == &aLobby) { return *this; } else { *m_pHead = aLobby.GetPlayer(); return *this; } } Lobby(const Lobby& aLobby) // Copy Constructor { cout << "Copy Constructor Called...\n\n"; m_pHead = 0; *m_pHead = aLobby.GetPlayer(); } private: Player* m_pHead;};void Lobby::AddPlayer(){ // create a new player node cout << "Please enter the name of the new player: "; string name; cin >> name; Player* pNewPlayer = new Player(name); // if list is empty, make head of list this new player if (m_pHead == 0) { m_pHead = pNewPlayer; } // otherwise find the end of the list and add the player there else { Player* pIter = m_pHead; while (pIter->GetNext() != 0) { pIter = pIter->GetNext(); } pIter->SetNext(pNewPlayer); }}void Lobby::RemovePlayer(){ if (m_pHead == 0) { cout << "The game lobby is empty. No one to remove!\n"; } else { Player* pTemp = m_pHead; m_pHead = m_pHead->GetNext(); delete pTemp; }}void Lobby::Clear(){ while (m_pHead != 0) { RemovePlayer(); }}ostream& operator<<(ostream& os, const Lobby& aLobby){ Player* pIter = aLobby.m_pHead; os << "\nHere's who's in the game lobby:\n"; if (pIter == 0) { os << "The lobby is empty.\n"; } else { while (pIter != 0) { os << pIter->GetName() << endl; pIter = pIter->GetNext(); } } return os;}void testCopy(Lobby aLobby);int main(){ Lobby mLobby; int choice; do { cout << mLobby; cout << "\nGame Lobby\n"; cout << "0 - Exit the program.\n"; cout << "1 - Add a player to the lobby\n"; cout << "2 - Remove a player from the lobby.\n"; cout << "3 - Clear the lobby.\n"; cout << "4 - Copy the Lobby.\n"; cout << endl << "Enter choice: "; cin >> choice; switch(choice) { case 0: cout << "Good-Bye\n"; break; case 1: mLobby.AddPlayer(); break; case 2: mLobby.RemovePlayer(); break; case 3: mLobby.Clear(); break; case 4: testCopy(mLobby); break; default: cout << "That was not a valid choice.\n"; } } while(choice != 0); return 0;}void testCopy(Lobby aLobby){ cout << "Copy Complete?\n\n"; cout << aLobby;}
You want the copied Lobby to contain copies of all of the Players in the source Lobby. The basic idea is ok (instead of copying a pointer value, we set the current Lobby head to point at a copy of the other Lobby's first Player), but the implementation is off.
First off, if we're going to do it by "*m_pHead = aLobby.GetPlayer();", then m_pHead has to already point at a Player object so that we can do the assignment. If it's uninitialized or null (and this is a problem in the copy constructor too - heck, it's explicitly set null first!), then the code blows up, because you deref a bad pointer.
Second, we need to make the copying work properly for Players first:
Oops, we need a copy constructor for the Player, now, too...
STOP.
Notice the similarity in structure here between what is done by assignment operators and copy constructors. It already looks like we're going to make a weird (and redundant!) mess of things if we try and implement each of them separately. Also, we have this annoying explicit 'delete' in the Player assignment op. Wouldn't it be great if we could implement one of these two things in terms of the other?
Enter the copy-and-swap idiom for assignment operators. As it turns out, it makes a lot more sense to implement the copy ctor "fully"; to do assignment, then, all we have to do is create some copy of the object (using the copy ctor, and thus making sure all the pointer ownership etc. stuff gets done properly) and then swap that data in to the current object. We can do that via std::swap, in most cases. This has the effect of setting all the data in the "current" object to the needed values from the copy, and stuffing the bits we're replacing into our temporary object. Finally, at the end of the assignment op code, the temporary falls out of scope, and its destructor is called - therefore cleaning up all the stuff we don't need any more. (Yes, you still have to write a destructor).
So first we will write a proper copy constructor for Players, and then use the copy-and-swap idiom to implement its assignment operator, replacing the mess above:
There are many advantages to this idiom. It is clear and exception-safe, and avoids duplication of code. It also automatically guards against self-assignment (the object makes a copy of itself, swaps itself with itself and destroys the copy; that's generally safe as long as your dtor and copy ctor are correct), and lets you make good use of initializer lists.
There's one last step here, of course, which is to make a proper destructor. Otherwise we'll be leaking memory ;)
I'm not going to lecture you about variable names, since I assume you picked up most of those conventions from the book you're reading out of. That can wait for another time. :)
Your homework:
1) Apply the provided techniques to write a proper copy constructor for the Lobby (hint: now that you have a proper Player copy constructor, you can rely one it), and use the copy-swap idiom to implement the assignment operator. Also write a proper destructor (this should be easier since you don't have "implicit list" semantics in the Lobby).
2) Research std::list and see how much easier you can make your life by using a std::list<Player> instead. Consider other containers as well, such as std::vector. Do some analysis on what kinds of operations you'll be doing on the Player "list", and choose the container appropriately.
3) Research the concept of pointer "ownership", and the Rule of Three. Googling around for these things should solidify the concepts in your mind.
First off, if we're going to do it by "*m_pHead = aLobby.GetPlayer();", then m_pHead has to already point at a Player object so that we can do the assignment. If it's uninitialized or null (and this is a problem in the copy constructor too - heck, it's explicitly set null first!), then the code blows up, because you deref a bad pointer.
Second, we need to make the copying work properly for Players first:
Player& Player::operator= (const Player& other) { // We need to make a copy of the subsequent players, as well - this happens // recursively; our base case is when the m_pNext is null. m_Name = other.m_Name; // the std::string copy ctor and assignment op already // work the way we want them to :) // Before we can copy subsequent players, we need to remove any Players the // current object might already be pointing at: delete m_pNext; // this is ok whether NULL or not. // We do this even if there are no players in the other list, so that the // lists end up being the same. if (other.m_pNext) { // then there is a list of players to copy. // Now we point our player list at a copy of the other player list. Since // we don't have a valid object to assign into, we have to use the copy // constructor on the "next" Player of the other: m_pNext = new Player(*(other.m_pNext)); } }Oops, we need a copy constructor for the Player, now, too...
STOP.
Notice the similarity in structure here between what is done by assignment operators and copy constructors. It already looks like we're going to make a weird (and redundant!) mess of things if we try and implement each of them separately. Also, we have this annoying explicit 'delete' in the Player assignment op. Wouldn't it be great if we could implement one of these two things in terms of the other?
Enter the copy-and-swap idiom for assignment operators. As it turns out, it makes a lot more sense to implement the copy ctor "fully"; to do assignment, then, all we have to do is create some copy of the object (using the copy ctor, and thus making sure all the pointer ownership etc. stuff gets done properly) and then swap that data in to the current object. We can do that via std::swap, in most cases. This has the effect of setting all the data in the "current" object to the needed values from the copy, and stuffing the bits we're replacing into our temporary object. Finally, at the end of the assignment op code, the temporary falls out of scope, and its destructor is called - therefore cleaning up all the stuff we don't need any more. (Yes, you still have to write a destructor).
So first we will write a proper copy constructor for Players, and then use the copy-and-swap idiom to implement its assignment operator, replacing the mess above:
// First attempt:Player::Player (const Player& other) { // Here is our code to copy a Player. // First, copy the name: m_Name = other.m_Name; // and now, copy the subsequent players. Since 'this' is a new Player, we // know there isn't anything in m_pNext that needs to be cleaned up. if (other.m_pNext) { m_pNext = new Player(*(other.m_pNext)); } else { m_pNext = 0; // make sure it's nulled out. }}// That's not ideal, though: we're only setting data members, so we really// ought to use an initializer list. Here's one place in C++ where the ternary// op really comes in handy ;)Player::Player (const Player& other) : m_Name(other.mName), // Recursively copy if not null. m_pNext(other.m_pNext ? new Player(*(other.m_pNext)) : 0) {}// No code within the function any more. :)// Ok, now that we have a nice, neat copy constructor, let's make a nice, neat// assignment operator to go with it, via the copy-and-swap idiom:Player& Player::operator= (const Player& other) { Player tmp(other); std::swap(*this, other); return *this;}There are many advantages to this idiom. It is clear and exception-safe, and avoids duplication of code. It also automatically guards against self-assignment (the object makes a copy of itself, swaps itself with itself and destroys the copy; that's generally safe as long as your dtor and copy ctor are correct), and lets you make good use of initializer lists.
There's one last step here, of course, which is to make a proper destructor. Otherwise we'll be leaking memory ;)
Player::~Player() { // If the pointer is null, we're done. Otherwise, we need to delete the // player that we point at. Of course, that will trigger the destructor for // that Player, and so on, recursively. Which is all as it should be. Oh, and // since deleting a null pointer is safe (and does nothing), all we really // need is: delete m_pNext; // If you can't convince yourself that this works, go read up on recursion, // and try again. :)}I'm not going to lecture you about variable names, since I assume you picked up most of those conventions from the book you're reading out of. That can wait for another time. :)
Your homework:
1) Apply the provided techniques to write a proper copy constructor for the Lobby (hint: now that you have a proper Player copy constructor, you can rely one it), and use the copy-swap idiom to implement the assignment operator. Also write a proper destructor (this should be easier since you don't have "implicit list" semantics in the Lobby).
2) Research std::list and see how much easier you can make your life by using a std::list<Player> instead. Consider other containers as well, such as std::vector. Do some analysis on what kinds of operations you'll be doing on the Player "list", and choose the container appropriately.
3) Research the concept of pointer "ownership", and the Rule of Three. Googling around for these things should solidify the concepts in your mind.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
