Sign in to follow this  
kaprikawn

Deleting an object by pointer

Recommended Posts

I'm building my first game using C++ and SDL.

I have a SpriteGroup object that contains a vector of pointers of my Sprites :

std::vector <Sprite*> m_sprites;

Everytime I want to create a sprite I create the object as normal and add it's pointer to this vector e.g.

Sprite* enemy = new Enemy ();

m_sprites.push_back( enemy );

(It's a little more complicated than this, but this is the pertinent code).

And my update function in my main loop will loop through this vector and call the update function of each object in the vector (and all of the rest of the stuff that needs doing).

When one of these objects goes out-of-bounds I want to remove it.  I have functions which identifies when this occurs and calls a delete function.  This finds the position in the vector of the object I want to remove and erases it :

void SpriteGroup::remove( int spriteID ) {

  for( int i = 0; i < m_sprites.size(); i++ ) {
    if( m_sprites[i] -> getSpriteID() == spriteID ) {
      m_sprites.erase( m_sprites.begin() + i );
    }
  }
}
 
The issue is that I don't think the object is being destroyed, merely removed from the vector.  Wouldn't this cause a memory leak?  When I try calling delete on the pointer before I remove the vector element, it causes a segfault i.e. by adding 'delete m_sprites[i];' before the erase call.
 
So my question is, if I have the pointer to an object, how do I call delete on that object before I remove it from the vector?
 
Thanks in advance.

Share this post


Link to post
Share on other sites

In C++, the modern de jure way of indicating pointer ownership (er, well reall ownership of the underlying memory) is to use std::unique_ptr.  That means your std::vector<Sprite*> should really be std::vector<std::unique_ptr<Sprite>>.
 

using SpriteOwningPtr = std::unique_ptr<Sprite>;
using SpriteBag = std::vector<SpriteOwningPtr>;

// declaring your container
SpriteBag m_sprites;

// add a new sprite
m_sprites.emplace_back(new Enemy());

// Passing a non-owned pointer elsewhere
draw_sprite(m_sprites[i].get());

// remove a sprite, with deletion
void SpriteGroup::remove( int spriteID )
{
  m_sprites.erase(std::remove_if(std::begin(m_sprites), std::end(m_sprites),
                                 [spriteID](SpriteOwningPtr const& sprite)->bool {
                                     return sprite->getSpriteID() == spriteID;
                                 },
                  std::end(m_sprites));
}
Edited by Bregma

Share this post


Link to post
Share on other sites

Bregma has a good approach, albeit one that might be a little advanced for "For Beginners", without additional explanation.

 


 When I try calling delete on the pointer before I remove the vector element, it causes a segfault i.e. by adding 'delete m_sprites[i];' before the erase call.

Are there any additional pointers to the Sprites, apart from those held in the vector? Is SpriteGroup::remove(...) being called (directly or indirectly) from the Spite / Enemy instance itself? Are you (directly or indirectly) calling SpriteGroup::remove(...) while you are iterating over the same vector?

Edited by rip-off

Share this post


Link to post
Share on other sites

Thanks both, but I think I've solved it another way.

I think when I deleted the object, the renderer still had something to do with that object and that's what caused the segfault.  I solved it by marking the object for deletion in a member variable, then deleting at the beginning of my next update pass.

Share this post


Link to post
Share on other sites

I would still use std::vector<std::unique_ptr>> for m_sprites -- and start adopting this pattern for other containers which "own" the objects whose pointers they contain. Learn what the various C++ smart pointers do, and start thinking about object ownership and lifetime to decide which to use.

Share this post


Link to post
Share on other sites

I would also recommend using std::shared_ptr to manage the ownership of your allocated objects.  It will do the work of deleting the object at the appropriate time for you.  

Just take care that if you're going to have multiple objects pointing to other objects, just using shared_ptr might lead to circular dependencies.  In this case, you can use std::weak_ptr.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this