Pushing local objects into a list

Question

I have a class

class Invader
{
public:
    Invader();
    ~Invader();
public:
    void Init(InvaderTypes invadertype, CIw2DImage *AlienImage);
    void Update(float dt);
    void Render();
    void SetAlienImage(CIw2DImage *image){ ImageAlien = image; }

    void        setVisible(bool show)       { Visible = show; }
    bool        isVisible() const           { return Visible; }


    Iw2DSceneGraph::CSprite         *AlienSprite;
    Iw2DSceneGraph::CAtlas          *AlienAtals;
    CIw2DImage                      *ImageAlien;
    std::list<Bullet*>              *Bullets;
    CIwFMat2D                       Transform;              // Transform matrix

    bool                             Visible;                // Sprites visible state
    bool                             Canfire;
};


void Invader::Init(InvaderTypes invadertype, CIw2DImage *AlienImage)
{
    if (invadertype == InvaderTypes::TOP_ALIEN)
    {
        //SetAlienImage(AlienImage);
        mImageAlien = AlienImage;
        // Create EnemyTop atlas
        int frame_w = (int)(mImageAlien->GetWidth() / 2);
        int frame_h = (int)(mImageAlien->GetHeight());
        AlienAtals = new CAtlas(frame_w, frame_h, 2, mImageAlien);
        AlienSprite = new CSprite();
        AlienSprite->m_X = 0;
        AlienSprite->m_Y = 0;
        AlienSprite->SetAtlas(AlienAtals);
        AlienSprite->m_W = (float)AlienAtals->GetFrameWidth();
        AlienSprite->m_H = (float)AlienAtals->GetFrameHeight();
        AlienSprite->m_AnchorX = 0.5;
        AlienSprite->SetAnimDuration(2);
    }
    else if (invadertype == InvaderTypes::MIDDLE_ALIEN)
    {

    }
    else if (invadertype == InvaderTypes::LAST_ALIEN)
    {

    }


    Visible = true;
    Bullets = new std::list<Bullet*>();
    Canfire = true;
}



Invader::Invader()
    {

    }
    Invader::Invader(const Invader&other)
    {
        AlienAtals = new CAtlas();
        AlienSprite = new CSprite();
        *AlienAtals = *other.AlienAtals;
        *AlienSprite = *other.AlienSprite;
    }

I try to initialize it by:

list<Invader> *invaders = new list<Invader>();

    int spacing = 10;
for (int i = 0; i < 5; i++)
{
    Invader invader;
    invader.Init(TOP_ALIEN, gameResources->getAlienImageTop());
    invader.AlienSprite->m_X = 50 + spacing;
    invaders->push_back(invader);
    spacing += 50;
}

After pushing the object invader to the list, at the end the invaders list holds pointers that are not initialized. All the pointers got lost the references. I wonder why ?

Answer 1

The problem, I assume, is what happens in ~Invader(). Let's simplify the example a ton:

struct A {
    int* p;
    A() { p = new int(42); }
    ~A() { delete p; }
};

A just manages a pointer. When an A goes out of scope, that pointer gets deleted. Now, what happens when we do this:

list<A> objs;
{
    A newA;
    objs.push_back(newA);
    // newA deleted here
}
// objs has one element... but its pointer has been deleted!

The problem is that copying A (which push_back() does) just performs a shallow copy: we copy our pointer. But since A manages its own memory, we need to do a deep copy. That is:

A(const A& rhs)
: p(new int(*(rhs.p)))
{ }

That way, the copied A won't double delete the same pointer. With C++11, this can be much more easily managed with just:

struct A {
    std::shared_ptr<int> p;
    A() { p = std::make_shared<int>(42); }
    ~A() = default; // this line not even necessary
};

Here, copying A will copy the shared_ptr and both copies of A will have a valid object to point to. If you can't use C++11, you can still use boost::shared_ptr<T> for all your memory management needs. And if you can't use that, then you have to write a copy constructor that does a full copy of all your pointer elements.

Or, the simplest solution, would be to just make your container have pointers:

list<A*> objs;
objs.push_back(new A);

Then the "shallow copy" is the right thing to do, and all you need to do is remember to delete everything in the container at the end.

Answer 2

Your list contains Invader objects. When you store them in the list, the Invader copy-constructor is called and a copy of the variable invader is stored in the list.

Unless you define a copy-constructor, it will be default simply do a shallow-copy of your object. This may not be what you want. You should write an explicit copy-constructor to make sure that the Invader is copied correctly.

Another solution would be to dynamically allocate Invader objects using the new keyword and storing pointers in the list. If you do that, be careful to make sure that you call delete on the Invader objects in the list when you are done with them.