CODEWITHSUNDEEP
c++debugging
TheForgot3n1
TheForgot3n1

Reputation: 378

Why is c++ prematurely destroying my string variable?

This error is one of the most mysterious I've gotten in probably years. I've been trying things back and forth for hours now and can't understand it.

In the code below, I am trying to dynamically decide which derived class to instantiate. I use the function parameter particle_type in order to determine that.

But since 

Particle p;

is not allowed, since you have to initialize references, I am trying to do it with a pointer as below.

Particle* p;
if (particle_type == "Featherr") { //This is never called because of the extra "r" I put, so ignore this block
        p = &FeatherParticle(particle_name, position, rotation, velocity, gravityEffect, lifeLength, scale);
}
else {
    p = &Particle(particle_name, position, rotation, velocity, gravityEffect, lifeLength, scale);
}

Particle particle1 = *p;
Particle particle2 = Particle(particle_name, position, rotation, velocity, gravityEffect, lifeLength, scale);

particles.push_back(particle1);
particles.push_back(particle2);

Now the particle2 is just me debugging, but it highlights this bug* very well. I put a breakpoint at particles.push_back(particle1); and found that the particle object stored by particle1 was broken:

enter image description here

I do not understand why the string is set to "" here. I can only guess it has to do with the scoping, because if I set the string outside the else statement, it works. Which makes my jaw drop at how stupid this functionality is *which is why I think it may be a bug or near-bug functionality.

Upvotes: 0

Views: 73

Answers (1)

Remy Lebeau
Remy Lebeau

Reputation: 598134

You are setting p to point at temporary objects that are destroyed immediately after the assignment, thus leaving p dangling. You need to use new instead:

Particle* p;
if (particle_type == "Featherr") { //This is never called because of the extra "r" I put, so ignore this block
    p = new FeatherParticle(particle_name, position, rotation, velocity, gravityEffect, lifeLength, scale);
}
else {
    p = new Particle(particle_name, position, rotation, velocity, gravityEffect, lifeLength, scale);
}
...

But then, you would be slicing the object when assigning *p to particle1, so use a reference instead (or simply get rid of particle1 since you don't need it):

Particle &particle1 = *p;

But then, you are trying to push Particle objects into particles, not Particle* pointers. So you would slice particle1 anyway. Polymorphism only works when using pointers/references, so you should be storing Particle* pointers (which means using new for particle2 as well):

std::vector<Particle*> particles;

...

Particle *p;
...
Particle *particle2 = new Particle(particle_name, position, rotation, velocity, gravityEffect, lifeLength, scale);

particles.push_back(p);
particles.push_back(particle2);

And don't forget to delete anything you new when you are done (and make sure Particle has a virtual destructor):

for(size_t i = 0; i < particles.size(); ++i) {
    delete particles[i];
}

Or, in C++11 and later:

for(auto *p : particles) {
    delete p;
}

But, it would be better to use std::unique_ptr to handle that delete for you automatically. Use std::make_unique() (C++14 and later), don't use new directly at all if you can avoid it:

std::vector<std::unique_ptr<Particle>> particles;

...

std::unique_ptr<Particle> p;
if (particle_type == "Featherr") { //This is never called because of the extra "r" I put, so ignore this block
    p = std::make_unique<FeatherParticle>(particle_name, position, rotation, velocity, gravityEffect, lifeLength, scale);
    // or:
    // p.reset(new FeatherParticle(particle_name, position, rotation, velocity, gravityEffect, lifeLength, scale));
}
else {
    p = std::make_unique<Particle>(particle_name, position, rotation, velocity, gravityEffect, lifeLength, scale);
    // or:
    // p.reset(new Particle(particle_name, position, rotation, velocity, gravityEffect, lifeLength, scale));
}

auto particle2 = std::make_unique<Particle>(particle_name, position, rotation, velocity, gravityEffect, lifeLength, scale);
// or:
// std::unique_ptr<Particle> particle2(new Particle(particle_name, position, rotation, velocity, gravityEffect, lifeLength, scale));

particles.push_back(std::move(p));
particles.push_back(std::move(particle2));

Upvotes: 3

Related Questions