Jump to content
  • Advertisement
Sign in to follow this  
cptrnet

delete pointers in a vector

This topic is 4774 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I am working on tetris, I have a shape class that consists of a bunch of blocks. When I create the shape I make the number of blocks, all of them pointers, and put them in a vector so I can have reference to them. When I close the window, it gives me a memory address error. So I was messing around with the code. If I don't add it into the vecotr it still gives me the error, but if I shutdown the memory with the block method and delete the pointer it's fine. So I thought I could create a method in the shape class that iterates through everything in the vector and shuts it down then deletes it. It didn't work. Could there be an easier way to reference the blocks in the shape class, I need to have the blocks dynamic because there are different shapes. I tried to have an array but I don't think I did that right or it didn't work. My question is, is there a way to iterate through a vector and call the shutdown method and then delete that pointer?

Share this post


Link to post
Share on other sites
Advertisement
By vector, do you mean the vector template in STL? Assuming so, STL was not designed to handle vectors of pointers. However, that does not mean that you cannot use them.


// Create vector
std::vector <object*> list;
list.clear ();

// To add an object to the vector
object *newObject = new object;
list.push_back (newObject);

// To iterate through the vector
for (std::vector <object*>::iterator i = list.begin (); i != list.end (); ++i)
{
object *currentObject = *i;
// Do something with the object
}

// To free the vector
for (std::vector <object*>::iterator i = list.begin (); i != list.end (); ++i)
{
if (*i)
{
delete *i;
*i = 0;
}
}
list.clear ();

Share this post


Link to post
Share on other sites
Yah I thought that is how you I would do it, but its still giving me the memory address error. Like something not being shutdown right.

Share this post


Link to post
Share on other sites
i'm not sure if i understand the problem correctly, but it sounds like you'd appreciate the boost smart pointers. boost::shared_ptr is safe and easy to use with stl containers.

std::vector< boost::shared_ptr<myClass> > v;
v.push_back( boost::shared_ptr<myClass>( new myClass ) );

when the vector v goes out of scope it will call the destructor for each myClass in the vector.

Share this post


Link to post
Share on other sites
I would like to figure out why this is happening, before I use code that does it for me. Could there be just an easier way to do it with an array?

Share this post


Link to post
Share on other sites
The description of your problem is rather murky. There are a number of things that could cause a "memory address error". Can you show us all the relevant code?

skittleo - there is no need to check for null pointers before using delete. Deleting a null pointer is a no-op.

Share this post


Link to post
Share on other sites
Header

enum ShapeType
{
TYPE1 = 0,
};

class Block : public Impulse::Primitives::xQuad
{

private:

D3DCOLOR m_Color;
int m_Column, m_Row;

public:

Block(IDirect3DDevice9 *pD3DDevice, IDirect3DTexture9 *pTexture);

};

class Shape
{

private:

IDirect3DDevice9 *m_pD3DDevice;
ShapeType m_Type;
bool m_bIsActive;
float m_StartX, m_StartY;
std::vector<Block*> m_Blocks;
xSceneManager *m_smgr;
xTexture *m_txtBlueBlock;
D3DXVECTOR3 m_position;
D3DXVECTOR3 m_dimension;

public:

D3DXVECTOR3 velocity;
D3DXVECTOR3 rotation;

Shape(IDirect3DDevice9 *pD3DDevice, xSceneManager *smgr, ShapeType type);

void CreateType1(void);

void Update(void);

void MoveColumnLeft(void);
void MoveColumnRight(void);

void CalculateLocation(void);

bool IsActive(void);

void Shutdown(void);

};



Source

#include "Block.h"

Block::Block(IDirect3DDevice9 *pD3DDevice, IDirect3DTexture9 *pTexture) :
Impulse::Primitives::xQuad(pD3DDevice, 0.0f, 0.0f, 33.3f, 33.3f, D3DCOLOR_RGBA(0, 0, 255, 255))
{
m_Color = D3DCOLOR_RGBA(0, 0, 0, 255);
m_Column = m_Row = 0;
}

Shape::Shape(IDirect3DDevice9 *pD3DDevice, xSceneManager *smgr, ShapeType type)
{

m_Blocks.clear();

m_pD3DDevice = pD3DDevice;

m_smgr = smgr;

m_Type = type;

velocity = D3DXVECTOR3(0, 0, 0);
rotation = D3DXVECTOR3(0, 0, 0);

m_txtBlueBlock = new xTexture(m_pD3DDevice, "blueBlock.tga");

switch(m_Type)
{
case TYPE1:

CreateType1();

break;
}

}

void Shape::CreateType1(void)
{

m_bIsActive = true;

float XAdd = 33.3f;

Block *newBlock1 = new Block(m_pD3DDevice, m_txtBlueBlock->GetTexture());
Block *newBlock2 = new Block(m_pD3DDevice, m_txtBlueBlock->GetTexture());
Block *newBlock3 = new Block(m_pD3DDevice, m_txtBlueBlock->GetTexture());
Block *newBlock4 = new Block(m_pD3DDevice, m_txtBlueBlock->GetTexture());
Block *newBlock5 = new Block(m_pD3DDevice, m_txtBlueBlock->GetTexture());

newBlock1->SetAbsoluteTranslation(START_OFFSET_X, START_OFFSET_Y, 0.0f);
newBlock2->SetAbsoluteTranslation(START_OFFSET_X + XAdd, START_OFFSET_Y, 0.0f);
XAdd += 33.3f;
newBlock3->SetAbsoluteTranslation(START_OFFSET_X + XAdd, START_OFFSET_Y, 0.0f);
XAdd += 33.3f;
newBlock4->SetAbsoluteTranslation(START_OFFSET_X + XAdd, START_OFFSET_Y, 0.0f);
XAdd += 33.3f;
newBlock5->SetAbsoluteTranslation(START_OFFSET_X + XAdd, START_OFFSET_Y, 0.0f);

m_smgr->Add(newBlock1);
m_smgr->Add(newBlock2);
m_smgr->Add(newBlock3);
m_smgr->Add(newBlock4);
m_smgr->Add(newBlock5);

m_Blocks.push_back(newBlock1);
m_Blocks.push_back(newBlock2);
m_Blocks.push_back(newBlock3);
m_Blocks.push_back(newBlock4);
m_Blocks.push_back(newBlock5);



}

void Shape::Update(void)
{

std::vector<Block*>::iterator it = m_Blocks.begin();

for(; it != m_Blocks.end(); ++it)
{
D3DXVECTOR3 position = (*it)->GetPosition();
D3DXVECTOR3 dimension = (*it)->GetDimension();

//Check to see if block has hit the ground or another block
if(position.y + dimension.y >= (532.8f + START_OFFSET_Y))
{
m_bIsActive = false;
}

if(m_bIsActive)
{
(*it)->SetRelativeTranslation(velocity);
(*it)->SetRelativeRotation(rotation);
}
}

CalculateLocation();
}

void Shape::MoveColumnLeft(void)
{
if(m_position.x > START_OFFSET_X)
{
std::vector<Block*>::iterator it = m_Blocks.begin();

for(; it != m_Blocks.end(); ++it)
{
(*it)->SetRelativeTranslation(-33.3f, 0.0f, 0.0f);
}
CalculateLocation();
}
}
void Shape::MoveColumnRight(void)
{
if(m_dimension.x < (START_OFFSET_X + 400.0f) - 33.3f)
{
std::vector<Block*>::iterator it = m_Blocks.begin();

for(; it != m_Blocks.end(); ++it)
{
(*it)->SetRelativeTranslation(33.3f, 0.0f, 0.0f);
}
CalculateLocation();
}
}

void Shape::CalculateLocation(void)
{

if(m_Blocks.size() == 0)
{
return;
}

std::vector<Block*>::iterator it = m_Blocks.begin();

D3DXVECTOR3 position = (*it)->GetPosition();
D3DXVECTOR3 dimension = (*it)->GetDimension();

float X = position.x;
float Y = position.y;
float Width = position.x + dimension.x;
float Height = position.y + dimension.y;

++it;

for(; it != m_Blocks.end(); ++it)
{

position = (*it)->GetPosition();
dimension = (*it)->GetDimension();

if(position.x < X)
{
X = position.x;
}
if(position.y < Y)
{
Y = position.y;
}

if(position.x + dimension.x > Width)
{
Width = position.x + dimension.x;
}
if(position.y + dimension.y > Height)
{
Height = position.y + dimension.y;
}

}

m_position.x = X;
m_position.y = Y;
m_dimension.x = Width;
m_dimension.y = Height;
}

bool Shape::IsActive(void)
{
return m_bIsActive;
}

void Shape::Shutdown(void)
{

std::vector<Block*>::iterator it = m_Blocks.begin();

for(; it != m_Blocks.end(); ++it)
{
if((*it))
{
(*it)->Shutdown();
delete *it;
}
}

m_Blocks.clear();

m_txtBlueBlock->Shutdown();
delete m_txtBlueBlock;

}


Share this post


Link to post
Share on other sites
- You know you always have five blocks, so the vector doesn't really help you.
- Blocks aren't polymorphic, and you aren't sharing them between Shapes, so storing them by pointer is only adding needless complication.
- If the SceneManager also tries to delete those Blocks, you have a double-delete, and that's bad. The blocks logically belong to their owning Shape, so that's where their deletion should be handled. (This might not be your problem; we don't have the relevant code here.)
- It looks like you've got quite a bit of confusion and intermingling between presentation logic and 'physics' logic. That's bad.
- What's with this "Shutdown" method (which I infer is defined in Impulse::Primitives::xQuad as well)? It seems to be doing what a destructor is supposed to do. Also, generally speaking, if the xQuad version is not virtual, your Block might not be cleaned up properly (though I don't think it's a problem here, you seem to just add primitive members to the xQuad base). Could the problem be there (improper handling of the IDirect3D{foo} pointers)? How well tested is that class so far?

Share this post


Link to post
Share on other sites
The scenemanager doesn't shut down anything it just renders. All My other quads are working Such as the logo for the game, the main area where the blocks fall etc. I can shut those down, but I have to delete them. Yah the shutdown is just like the destructor. I have a Game programming RPG book and it seems to do that a lot. I have five blocks in there for now as TYPE1 shape, as I get this taken care of I can go onto shape TYPE2 which may only have four blocks, and If i want to make up some weird shape it could have any number of blocks. I comment the code out where it makes the new blocks, and the shutdown and it closes cleanly. If i just uncomment the newBlock1 it errors unless I shutdown in the local method. I need a way of referencing all the blocks in the shape so i know what row and column they are on.

Share this post


Link to post
Share on other sites
Quote:
Original post by skittleo
By vector, do you mean the vector template in STL? Assuming so, STL was not designed to handle vectors of pointers.


Baloney [smile] The STL uses template types for a reason.

in your .h/.hpp file:


#include <vector>
#include <algorithm>

class Delete{

public:

template<typename T>
inline void operator()(const T* ptr) const
{
delete ptr;
}
};




in your .cpp file:


// assume v_blocks is your vector of (block *)

std::for_each(v_blocks.begin(), v_blocks.end(), Delete()); // deletes all objects pointed at by contained ptrs



Of course, the preferred method of holding ptrs in STL containers would be to use smart pointers... in that case, the for_each/Delete idiom isn't needed.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!