C++ Odd behaviour with vector populated with constructor

Question

I'm experiencing some "odd behaviour" with populating a vector value with the use of the constructor.

So, I have this block class that populates this static vector called blockList whenever the constructor is called.

static std::vector<Block*> blockList; Block::Block(float x, float y) : RectangleShape(sf::Vector2f(WIDTH, HEIGHT)) { blockList.push_back(this); setFillColor(sf::Color(221, 221, 221, 255)); setPosition(x, y); }

Now, when I populate the values in another class I have called "Game" ; in its constructor like so:

Game::Game(sf::RenderWindow& window) { // other code here Block firstBlock(10, 10); // possible more blocks here }

and then attempt to draw, it crashed. In addition, the coordinates of this block print out to be (x: 9.8439e-12, y: 9.84394e-12). Now what I've tried doing is throwing it in the draw method (which I'm aware is bad as it's constantly being called but for debugging purposes):

void Game::drawObjects() { // draw other shapes here Block oneBlock(50.0f, 50.0f); std::cout << std::string(10, '-') << std::endl; for (Block* block : Block::getBlockList()) { std::cout << block->getPosition().x << " - " << block->getPosition().y << std::endl; window->draw(*block); } std::cout << std::string(10, '-') << std::endl; }

The coordinates and the drawing of the object work perfectly (even if the vector is being populated heavily). So I'm wondering why this is happening and a possible way to work around this. I don't like the idea, but perhaps I should have flag to detect whether or not they've been created and then create them in the draw method?

Did you happen to read the compiler warnings? They're important. — 3Dave
– 3Dave, Commented Jul 19, 2016 at 3:24
Also, you don't show any mechanism which removes blocks in their destructor. Stop being clever, and just add a ::Add and ::Remove method. — 3Dave
– 3Dave, Commented Jul 19, 2016 at 3:27
@DavidLively No there aren't any warnings as it appears to be a logical error. — Jujunol
– Jujunol, Commented Jul 19, 2016 at 3:40

songyuanyao · Accepted Answer · 2016-07-19 03:23:13Z

So I'm wondering why this is happening

In Game::Game(), Block firstBlock(10, 10); means firstBlock is a local object which will be destroyed when get out of the scope of Game::Game(). Then the pointer saved by blockList.push_back(this); in blockList becomes dangled pointer, and access it leads to UB.

and a possible way to work around this.

Do you really need a vector of pointer here? You could just

static std::vector<Block> blockList;

then use it like:

Block firstBlock(10, 10); blockList.push_back(firstBlock);

Or they could use std::shared_ptr<Block>, which might be "better". I also suspect that Block::~Block() doesn't delete the pointer. Personally, I'd make the blockList a distinct class with explicit add and remove methods.
@KenY-N If pointer is necessary then smart pointers will be better. Anyway making a distince blockList is a good idea.
I put "better" in quotes as I felt "less worse" would have been more accurate but too blunt.

Ped7g · Accepted Answer · 2016-07-19 10:39:00Z

As addendum to songyuanyao answer.

You should think a bit what is owned by what in your app.

I think Game can own all the blocks, so having vector<Block> blocks; inside Game is IMO OK.

Or if there's lot of stuff, and you want to keep Game a bit shorter, you may introduce some World wrapper class owning the blocks, then Game has among other things single World world; instance, which contains the blocks vector.

Avoid naked pointers in the source, they often just point out, that you are not sure who should own the instance of the object. Having everything "belonging somewhere" will make all the memory releasing just question of going out of scope. For example if your main has Game game; local instance, it will get completely destructed upon exiting main (releasing also the inner world instance with the vector of all blocks).

Then the C++ feels almost like having GC, but you don't need to be afraid with when the resource releasing happens, as you make that explicit by exiting the particular scope.

Collectives™ on Stack Overflow

C++ Odd behaviour with vector populated with constructor

2 Answers 2

3 Comments

Comments

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

3 Comments

Comments

Related