• 11
• 27
• 9
• 20
• 31

# Question regarding references.

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

## Recommended Posts

Hey again. :) Been working on my text-based RPG, which I mentioned in my other thread and I've come across a situation where I need a little help.
CRoom CRoomManager::FindRoom(int findID)
{
CRoom *temp = 0;

for (vector<CRoom>::size_type size = 0; size != this->RoomList.size(); size++)
{
if (this->RoomList[size].RoomID == findID)
temp = &(this->RoomList[size]);
}

return temp;
}

void CRoomManager::SetStartingRoom()
{
// To do...
}

void CRoomManager::SetCurrentRoom(int newID)
{
// To do...
}


This is what I have at the moment. The problem is, I don't know what I should be returning from FindRoom(). I want to be able to call FindRoom() from inside SetStartingRoom() and SetCurrentRoom(). I'd also like to be able to check if a room is found, like so: Edit: Helps if the source is right (below).
void CRoomManager::SetCurrentRoom(int newID)
{
CRoom temp = FindRoom(newID);
if (temp != 0)
this->CurrentRoom = temp;
}


That's why I've used CRoom *temp = 0; as I was hoping to test against a null pointer. I didn't seem to be able to get this working without using pretty ugly syntax for something which I'm sure is a pretty common task. So, my question is, should I be returning by reference here and changing it to something like:
CRoom& CRoomManager::FindRoom(int findID)
{
CRoom temp;

for (vector<CRoom>::size_type size = 0; size != this->RoomList.size(); size++)
{
if (this->RoomList[size].RoomID == findID)
temp = this->RoomList[size];
}

return temp;
}


Bit confused (which is why I'm writing the RPG in the first place) as temp would surely get destroyed when FindRoom() returns, so I don't know where this leaves me. Would appreciate any help. :)

##### Share on other sites
So correct me if I'm wrong, but what I think you're trying to do is create a bunch of rooms with a unique ID. Then you're trying to create a function that returns a room when given an ID, right? If that's the case, you should use a map to store your (ID, room) pairs. Then, your FindRoom function will be given an ID which it then can use to search through the map to find the correct Room, which it will then return.

Also, since you're making a text RPG, another way to store your "Rooms" in a sequential order is to use a Linked List. Just thought I'd mention it.

##### Share on other sites
Never used a map before, but I've just done some research and it looks like you might be right. Had no idea that existed, thanks!

I'll post back if I end up having fun with that. ;)

##### Share on other sites
Quote:
 Original post by SeymourClearlyNever used a map before, but I've just done some research and it looks like you might be right. Had no idea that existed, thanks!I'll post back if I end up having fun with that. ;)

Also, if you wanted multiple Rooms with the same ID, you can use a multimap.

Have fun learning something new!

##### Share on other sites
Quote:
 Original post by SeymourClearlyNever used a map before, but I've just done some research and it looks like you might be right. Had no idea that existed, thanks!I'll post back if I end up having fun with that. ;)

You'll run into same problem. What happens if room with a given ID is not found?

If you have a vector, room index is implied:
std::vector<Room> rooms.// To find room by a given id:rooms[id];

Just insert them properly, so that RoomID matches the index they're stored in.

Ideally, you'd do something like this:
class RoomManager {  void addRoom( Room & room )  {    rooms.resize(room.RoomID); // make sure we have space    rooms[room.RoomID] = room;  }  Room & operator[]( unsigned int index )  {    if (index >= rooms.size()) throw std::exception("Invalid room");    return rooms[index];  }  std::vector<Room> rooms;};...RoomManager rm;...assert( rm[17].RoomID == 17 );

Also, drop the C prefix from class names.

##### Share on other sites
Thanks for the posts.

I have managed to change it over to a map and because of it, I don't even need the FindRoom method. Here's what I've changed it to:

void CRoomManager::SetStartingRoom(int roomID){	this->CurrentRoom = RoomList[roomID];}void CRoomManager::SetCurrentRoom(int roomID){	this->CurrentRoom = RoomList[roomID];}

Obviously no error checking at this stage but I need to re-work a few things now so I'll have to add that to the list.

@ Antheus,

Thanks, overloaded operators are next on my list, so that gives me a good head's up. One thing though, what's wrong with the C class prefix? Is there a technical/clarity reason behind your suggestion or is it merely personal preference?

I'll take all suggestions I can get at this point but I'm curious as to what the reason is.

Cheers for the help! :)

##### Share on other sites
Quote:
 Original post by SeymourClearlyThanks for the posts.I have managed to change it over to a map and because of it, I don't even need the FindRoom method. Here's what I've changed it to:*** Source Snippet Removed ***Obviously no error checking at this stage but I need to re-work a few things now so I'll have to add that to the list.

That won't work, since map has overloaded operator[] which has some side-effects. If object isn't found, you get undefined behavior. Application may crash, throw an exception, ugly stuff.

The map find idiom is this:
typedef std::map<int, std::string> MyMap; // whateverMyMap map; // instanceMyMap::iterator i = map.find( 17 );if (i == map.end()) {  // not found} else {  i->second; // the std::string we need}

Quote:
 One thing though, what's wrong with the C class prefix? Is there a technical/clarity reason behind your suggestion or is it merely personal preference?

WBecause Wyou Wknow Weach Wof Wthe Wthings Win Wthis Wsentence Wis Wa Wword Weven Wwithout Wprefix.

Look ugly, doesn't it?

##### Share on other sites
Haha, you've got me there mate. Cheers for that and for pointing out the problem with my method. :)

##### Share on other sites
Quote:
 Original post by AntheusThat won't work, since map has overloaded operator[] which has some side-effects. If object isn't found, you get undefined behavior. Application may crash, throw an exception, ugly stuff.
Are you positive about that? I was under the impression that map::operator[]() inserts the element if it does not already exist.

(@The OP: Even if that is the case, your code may not do exactly what you expect; be sure to check out Antheus' example of how to check to see if a map contains a specific item.)

##### Share on other sites
Thanks jyk,

I believe you're right about the behaviour of the [] operator. Directly from Accelerated C++:

Quote:
 When we index a map with a key that has not yet been seen, the map automatically creates a new element with that key. That element is value-initialised.

That's actually why I was using the method I did, so it's nice to know I've got to be a little bit more careful.