Why's my vector element all garbage?

Question

I have a GameLobby class that keeps a list of the currently active games. I want to fetch the active games from a GameLobby (singleton) object, and display these to the user.

(Disclaimer: I'm pretty new to C++, so the following code isn't exactly stellar. Neither is it the complete code, but I feel confident that all the relevant instructions have been included.)

First some definitions

class GamesMenu : public MyGameLayer
{
private:
    std::vector<Game*>* _activeGames;
    void displayGamesList();
    void refreshGamesList();
};

and

class MyGameLayer : public cocos2d::CCLayer
{
private:
    GameLobby* _gameLobby;
public:
    GameLobby* getGameLobby();
};

and

GameLobby* MyGameLayer::getGameLobby()
{
    return _gameLobby;
}

Now to the problem at hand. I want to execute GamesMenu::refreshGamesList() which looks like this:

void GamesMenu::refreshGamesList()
{
    GameLobby* gameLobby = getGameLobby();
    if (gameLobby) {
        _activeGames = gameLobby->getActiveGames();
        Game* game = _activeGames->at(0); // For debug purposes only - this game is NOT garbage
    }
    displayGamesList();
}

where

std::vector<Game*>* GameLobby::getActiveGames()
{
    if (_loggedInPlayer) {
        refreshActiveGames(_loggedInPlayer->GetPlayerToken());
    } else {
        refreshActiveGames("");
    }
    return &_activeGames;
};

and std::vector<Game*> _activeGames is a private member of GameLobby.

However, when execution hits displayGamesList(), things get pretty bad

void GamesMenu::displayGamesList()
{
    for (unsigned i = 0; i < _activeGames->size(); i++) {
        Game* game = _activeGames->at(i); // The contents of game is garbage. Why?
        std::string opponentName = game->GetOpponentName(); // This I don't even want to talk about 
    };
    /* Supressed drawing stuff */
}

When I inspect game in GamesMenu::refreshGamesList, the contents of game seems fine. When I inspect game in GamesMenu::displayGamesList, the contents is all garbage. It is as if the elements of the vector points to the wrong data or something.

Can anyone please help me untangle myself out of this mess? Thanks! :)

The fact that you are using pointers makes me a little uncomfortable. It makes me wonder where vector is being allocated and it complicates the memory management. — trojanfoe, Aug 18 '13 at 08:20
[A good introductory C++ book](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) will talk about `std::vector`s and how to use them properly, so I highly recommend that you pick such a book up. While you're at it, [stop using pointers so much](http://klmr.me/slides/modern%2Dcpp.pdf). Kudos to you for using the standard containers though. — In silico, Aug 18 '13 at 08:23
Yeah, that surely adds a level of abstraction. The reason I do this, is that I tried to copy the vector. But this didn't work for me at all. I didn't get any sensible result from my attempt at 'std::vector GameLobby::getActiveGames()'. I'm not sure why, but suspecting it might be related to elements not being copied correctly(?). Basically what I wonder is: why can I see the contents of the vector in one method and not in the next? Nothing significant happened between the two calls ... — conciliator, Aug 18 '13 at 08:27
If you store things in a vector, returning the address of that vector is the wrong thing to do. Return a const reference to the vector, assuming you don't intent to modify it. — Mats Petersson, Aug 18 '13 at 08:35
Your code snippets doesn't show the declaration or initialization of `_activeGames`. I suspect what happens is that your pointer goes stale (points to data that is no longer available) in some way. — Mats Petersson, Aug 18 '13 at 08:38
@MatsPetersson that definitely seems like a sound advice - I'm not going to modify the vector. Regarding the snippets, I tried to keep the post as small as possible, but I think I'll want to update it later today. — conciliator, Aug 18 '13 at 08:42
Shouldn't the return value of `getActiveGames()` be only `_activeGames` instead of `&_activeGames` ? — Uchia Itachi, Aug 18 '13 at 08:52
@UchiaItachi I think `GameLobby` is the home of the actual *instance* (not just a pointer to) the active games vector. Consequently, returning its address makes sense. What I don't understand is, if `GameMenu` is opened from a `GameLobby`, why doesn't the menu just always use the lobby's `getActiveGames()` member and keep *no* `_activeGames` pointer in its instance whatsoever. — WhozCraig, Aug 18 '13 at 08:55
@WhozCraig yup, that's why. Regarding keeping a pointer to '_activeGames' in 'GameLobby': don't really need to, no. I'm going to try access it directly. — conciliator, Aug 18 '13 at 09:02
@conciliator I meant get rid of the pointer in GameMenu (since it has a pointer to the lobby, and thus access to its `getActiveGames()` exposure.) — WhozCraig, Aug 18 '13 at 09:05
@WhozCraig wow man - that did it! (Unfortunately, I'm still in dark as to why the pointer '_activeGames' didn't do what I thought it should've done.) — conciliator, Aug 18 '13 at 09:05
@conciliator Without seeing the constructor for GamesMenu and how that variable is initialized and maintained I would have difficulty speculating *exactly* why it works, but it was likely in that area. Glad its working. — WhozCraig, Aug 18 '13 at 09:07
check the following method ` if (_loggedInPlayer) { refreshActiveGames(_loggedInPlayer->GetPlayerToken()); } else { refreshActiveGames("");` check the value of _loggedInPlayer and then check refreshActiveGames check to see if loggedInPlayer is being set also check ` if (gameLobby) ` as I think that gameLobby is probably not being set and you are returning a reference to an uninitialized vector. You need to share your code for populating the vector and vet that out to make sure it is being called — bjackfly, Aug 18 '13 at 10:10

Why's my vector element all garbage?

0 Answers0

Linked