0

In my header file I have:

class Game
{
private:
    string _name;
    Level _currentLevel;
public:
    Game();
    ~Game();
    void setName();
    void run();
};

In my cpp file I have my run function:

void Game::run()
{

    bool finished = false;
    string input;
    while (!finished)
    {
        // get input
        std::cout << "Enter a command: \n";
        std::getline(std::cin, input);
        if (input == "quit")
        {
            finished = true;
        }
        else if (input == "new")
        {
            Level _currentLevel;
        }
        else if (input == "print")
        {
            _currentLevel.printMap();
        }
        else
        {
            std::cout << "Unknown command! \n";
        }


    }
}

constructor and printmap method of Level

Level::Level()
{
    _width = RandomGenerator::Instance()->getRandom(6, 10);
    _height = RandomGenerator::Instance()->getRandom(6, 10);
    for (int y = 0; y < _height; y++)
    {
        for (int x = 0; x < _width; x++)
        {
            addRoom(x, y);
        }
    }
}

void Level::printMap()
{
    for (int y = 0; y < _height; y++)
    {
        for (int x = 0; x < _width; x++)
        {
            if (x != 0)
                cout << " - ";
            cout  << _map[coordinate(x, y)].getSize();
        }
        cout << "\n";
    }
}

However when I type new, that runs Level _currentLevel; (to create a new non pointer object), the object dosnt change. I can see it dosn't change the values of level when I run printmap (which prints a map with 30 random values created in the Level constructor). While debugging the value of _height changes in the Level constructor. How should the value of _currentLevel be updated from my Game class?

Sven van den Boogaart
  • 11,833
  • 21
  • 86
  • 169

2 Answers2

3

Your new block creates a local stack variable that happens to have the same name as your instance variable (_currentLevel). It does not overwrite the instance variable, and that's why nothing changes.

You have a few straightforward choices:

  • Use a pointer. I suggest using a shared_ptr so you don't have to worry about deallocating memory on your own.

  • Extend Level to have an Initialize function. The constructor can call this, or you can call it from other code later if you want to re-initialize an existing variable.

  • Copy a new local variable to the instance variable.

Personally, I'd suggest the pointer, but either works.

Donnie
  • 45,732
  • 10
  • 64
  • 86
  • Thanks you pointed out what i did wrong, create two values witht the same name. I solved it now by "Level newLevel; _currentLevel = newLevel;" I should try avoiding pointers as suggested here right ? http://stackoverflow.com/a/22146244/1667868 – Sven van den Boogaart Jan 09 '16 at 23:45
  • 1
    Does a `shard_ptr` deal with fragmented memory? (Sorry, couldn't resist; wonderful typo!) – Pete Becker Jan 09 '16 at 23:46
  • If `_currentLevel` lives in this object, and will never get stored outside of it, what you're doing is probably ok. Note that your current fix copies the local into the object (and then you destroy the local when it goes out of scope). if at some point in the future you need to pass `_currentLevel` out, with your current implementation you'd only be able to pass out a copy safely. – Donnie Jan 09 '16 at 23:54
  • 1
    @Sven B Avoiding pointers at all cost is like fearing your own shadow. Pointers are an amazing tool and very useful. Learning how and when to use them is a better approach than thinking that you have to avoid them. They are your friends, not your enemies. – m_pOatrix Jan 10 '16 at 00:03
0

For starters the prototypes should be in the header and the implementation in the source file. Apart from that, in game::run you declare a second local _currentlevel which shadows the class variable. Outside the constructor, you never modify the class field.

Replace the shadowing line with this->_currentlevel = Level();

bgarcia
  • 154
  • 6