delete pointers in a vector

Started by
11 comments, last by Verg 18 years, 9 months ago
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?
If you insist on saying "DUH", try to make a post that makes a little more sense
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 vectorstd::vector <object*> list;list.clear ();// To add an object to the vectorobject *newObject = new object;list.push_back (newObject);// To iterate through the vectorfor (std::vector <object*>::iterator i = list.begin (); i != list.end (); ++i){   object *currentObject = *i;   // Do something with the object}// To free the vectorfor (std::vector <object*>::iterator i = list.begin (); i != list.end (); ++i){   if (*i)   {      delete *i;      *i = 0;   }}list.clear ();
....[size="1"]Brent Gunning
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.
If you insist on saying "DUH", try to make a post that makes a little more sense
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.
This space for rent.
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?
If you insist on saying "DUH", try to make a post that makes a little more sense
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.
"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." — Brian W. Kernighan
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;}
If you insist on saying "DUH", try to make a post that makes a little more sense
- 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?
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.
If you insist on saying "DUH", try to make a post that makes a little more sense
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.
my_life:          nop          jmp my_life
[ Keep track of your TDD cycle using "The Death Star" ] [ Verge Video Editor Support Forums ] [ Principles of Verg-o-nomics ] [ "t00t-orials" ]

This topic is closed to new replies.

Advertisement