Public Group

# Book exercise. Is it correct?

This topic is 3742 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

Hello. I have finished the chapter on Advanced Classes of Beginning C++ through game programming. The last program of the chapter is this:
#include <iostream>
#include <string>
using namespace std;

class Player
{
public:
Player( const string& name = "" );
string GetName() const;
Player* GetNext() const;
void SetNext( Player* next );

private:
string m_Name;
Player* m_pNext;
};

Player::Player( const string& name ): m_Name( name ), m_pNext( 0 )
{
}

string Player::GetName() const
{
return m_Name;
}

Player* Player::GetNext() const
{
return m_pNext;
}

void Player::SetNext(Player* next)
{
m_pNext = next;
}

class Lobby
{
friend ostream& operator << ( ostream& os, const Lobby& aLobby );

public:
Lobby();
~Lobby();
void RemovePlayer();
void Clear();

private:
};

{
}

Lobby::~Lobby()
{
Clear();
}

{
//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

//otherwise find the end of the list and add the player there
else
{
while( pIter->GetNext() != 0 )
{
pIter = pIter->GetNext();
}
pIter->SetNext( pNewPlayer );
}
}

void Lobby::RemovePlayer()
{
{
cout << "The lobby is empty. No one to remove.\n";
}
else
{
delete pTemp;
}
}

void Lobby::Clear()
{
{
RemovePlayer();
}
}

ostream& operator << ( ostream& os, const Lobby& aLobby )
{

os << "\nHere's who's in the lobby:\n";
if( pIter == 0 )
os << "The lobby is empty.\n";
else
{
while( pIter != 0 )
{
os << pIter->GetName();
pIter = pIter->GetNext();
}
}

return os;
}

int main()
{
Lobby myLobby;
int choice;

do
{
cout << myLobby;
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 << "\nEnter choice: ";
cin >> choice;

switch( choice )
{
case 0: cout << "Good bye.\n"; break;
case 2: myLobby.RemovePlayer(); break;
case 3: myLobby.Clear(); break;
default: cout << "That was not a valid choice.\n";
}
} while( choice != 0 );

system( "PAUSE" );
return 0;
}


The author says that the Lobby::AddPlayer() function is inefficient because it iterates through all the objects to add one. So the exercise is to make it efficient by adding an m_pTail member function. This is what I've done:
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
{
m_pTail = pNewPlayer;
}

//otherwise find the end of the list and add the player there
else
{
m_pTail->SetNext( pNewPlayer );
m_pTail = pNewPlayer;
}
}


Is it correct? Thanks for your time.

##### Share on other sites
I think you posted the wrong function...

Ouch. Edited.

##### Share on other sites
Heh, I remember this exercise from the book. I think that looks correct. If you're using VStudio of some sort, can always debug and follow it through. However I found with c++ that the debugger leaps through the vector classes and stuff, making it very annoying to debug.

##### Share on other sites
Did you update void Lobby::RemovePlayer() to update m_pTail? If not, then if you add, remove and add a player, the application will either crash or produce weird results.

##### Share on other sites
Quote:
 Original post by AntheusDid you update void Lobby::RemovePlayer() to update m_pTail? If not, then if you add, remove and add a player, the application will either crash or produce weird results.

Should I? It's the first person who's being removed, not the last. :)

Correct me if I'm wrong.

##### Share on other sites
There are some issues. It would appear that it would work correctly (although it would be really nice if the tail pointer was set to 0 if the list is emptied by a call to RemovePlayer, as a C++ programmer you will learn to hate dangling pointers).

Firstly, unless the order of the players in the lobby is important for some reason, you could insert the new player at the start of the list, eliminating the need to hold a reference to the tail.

The more pressing issue is of class responsibility. While I understand that the book might be trying to demonstrate the concepts of a linked list, it is still important to think about them the right way. We can separate the "linked list" from the "things in the list" fairly easily:
class Player{public:	Player( const string& name = "" );	string GetName() const;private:	string m_Name;};class PlayerList{    PlayerList();    ~PlayerList();    // both these functions could take an integer parameter    // this value would indicate where to insert or remove    void insert(const Player &);    void remove();    void clear();private:    struct PlayerNode    {        Player player;        PlayerNode *next;    };    PlayerNode *head;};class Lobby{     // operations     // can't implement operator<< yetprivate:     PlayerList players;};

Each class now has a defined task. The Player only cares about a single players information, it doesn't need to know about (or be responsible for) any other players. The PlayerList class is responsible for holding an arbitrary number of players. It uses the inner structure PlayerNode to represent the links in the list. Finally, the Lobby is an extension of the user interface (I suppose), that prompts the user for information, etc(it could be removed, and main could interact with the PlayerList directly).

While this is more complicated looking, this has benefits that aren't immediately obvious. For example, with your current implementation, anyone who has a reference to a Player object can modify the list the player is in! This is obviously undesirable, it could cause no end of bugs.

The above could be simplified using templates, but that truly is for later. Also, the above does not allow one to iterate over the PlayerList and do something with each player, this is actually relatively easy to implement but I'll leave it out of the interface for the moment.

If you are interested in going beyond the task you have been given, then you could implement the above to the best of your ability. Most of it involves moving code from the lobby into the PlayerList. From there, we could make more suggestions to expand the interface of PlayerList to a fully functional linked list. Writing your own linked list is always an important moment for every aspiring programmer.

Once you have written your own version, you will be able to throw it away and learn the joy that is std::list<>. [smile]

##### Share on other sites
Quote:
Original post by sheep19
Quote:
 Original post by AntheusDid you update void Lobby::RemovePlayer() to update m_pTail? If not, then if you add, remove and add a player, the application will either crash or produce weird results.

Should I? It's the first person who's being removed, not the last. :)

Correct me if I'm wrong.

It's ok, I wrote too fast. I had a generic linked list remove in mind.

• 10
• 16
• 14
• 18
• 15