Vector erase with pointers?

Calling erase() on an item in a vector does not automatically delete the item pointed to. It's up to you to delete the memory allocated for your things. Here's a simple example to demonstrate that:

#include <iostream>
#include <vector>

using namespace std;

int g_id = 0;

class Thing {
public:
	Thing() {
		id = g_id++;
	}

	~Thing() {
		cout << "~Thing " << id << endl;
	}

private:
	int id;
};

vector<thing*> g_Things;

int main ( int argc, char *argv[] ) {

	for ( int i = 0; i < 10; ++i ) {
		Thing* t = new Thing;
		g_Things.push_back(t);
	}

	for ( int j = 9; j >= 0; --j ) {
		/* delete *( g_Things.begin() + j ); */
		g_Things.erase( g_Things.begin() + j );
	}

	return 0;
}

If you compile this code you'll see that nothing is printed. The destructor is never called. Now uncomment the delete statement on line 33 and try it again.

Hope this helps,

Tony


[edited by - AnthonyLewis on January 21, 2004 11:40:06 PM]

its reasons such as this which led me to using boost::shared_ptr with my containers, small objects copys when moving pointers from one container to the other and it can still be treated like a pointer with automagic destruction when a list/vector/whatever goes out of scope

Neither
delete *( g_Things.begin() + j );
nor
delete g_Things[j];
work. They both compile but at runtime I get a NullReferenceException. I don''t want to use any libraries (i.e. boost) so what are my choices here?

Okay I''m not sure -exactly- what''s going on, but my hunch is that the destructor is getting called too many times.

Whenever you push_back a thing to a vector, it makes a copy of the object, using the object''s copy constructor. (side note about copy constructors: if you don''t write one, the compiler will write one for you, and it just does a shallow copy). But then these objects that were copy constructed are destroyed as normal, so there are certain ID values that get destroyed several times.

Even if that''s not the exact problem, I think that your strategy of having a global vector & having objects add and remove themselves from the global vector, is not a great design. I would try to find a different way to solve whatever problem you are solving.

Hand the thing to be deleted off to a pointer, erase the item from the vector, and delete the pointer. Don't delete a reference.

#include <iostream>
#include <string>
#include <vector>

int main()
{

//Allocate

int *ptr = new int [100];
std::vector <int*> vec;
vec.push_back(ptr);

//Delete

int *ptrToItemToBeDeleted = vec[0];
vec.erase(vec.begin());
delete [] ptrToItemToBeDeleted;

} 

Edit: code tag sucks

[edited by - invective on January 22, 2004 7:04:35 PM]

THING* blah = g_Things[g_Things[i+0]->ID];
g_Things.erase(g_Things.begin() + g_Things[i+0]->ID);
delete blah;

still gives a NullReferenceException

Edit: Damn [ i ] gives italics

[edited by - ms291052 on January 22, 2004 7:39:22 PM]

Just a clarification: A NullReferenceException is a Managed exception. Are you using managed C++ on purpose?

Yeah, I know it''s managed... I just used .NET (managed is therefore the default). Is managed bad for some reason I don''t know or what? Does that matter? I don''t imagine that has any effect on the actual code or anything...

the main difference is managed code is compiled down to the .Net bytecode (forgot its proper name)

out of intrest, why dont u want to use something like boost?
btw, arguements about 'its slow' or 'bloated' wont wash unless you can prove it

[edited by - _the_phantom_ on January 22, 2004 10:12:43 PM]

Because I take pride in doing my work with no libraries (save DX and win32 and all that standard ish, of course :-p).

Managed? With A Garbage Collector?

If so, you need not delete memory in the first place, that is the GC''s job.

The way it works is that if a block of mem is no longer pointed to, or referenced, it will deleted by the GC automagically.

---

blah blah blah.

Is managed GC''d? I didn''t think so... If it is then w00t, but I''m still getting damned errors about bad pointers.

quote:

---

Original post by ms291052
Because I take pride in doing my work with no libraries (save DX and win32 and all that standard ish, of course :-p).

---

ah, one of THOSE programmers, the kind to ignore a good solution because they didnt write it and instead prefer to have lots of problems, i see.
good good, carry on.

quote:

---

Original post by ms291052
Because I take pride in doing my work with no libraries (save DX and win32 and all that standard ish, of course :-p).

---

You do know that the people who started the Boost project actually are the very same people who decide what is and what isn't "standard ish" where C++ is concerned, right ? And that its purpose is to prepare future additions to the C++ standard library.

Ah. I thought so.

---

“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 (C programming language co-inventor)

[edited by - Fruny on January 23, 2004 1:05:03 AM]

Ugh. I don''t want to use boost. (period). Can anyone just help me fix my problem?

Yes. Either do the loop by hand or write a custom remove_delete_pointer (and remove_delete_array, remove_delete_pointer_if, remove_delete_array_if) functor to do it STL-style.

The code to write should be obvious.

edit: At any rate, if the Thing* is NULL, remove(begin, end,0) ought to work just fine.

---

“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 (C programming language co-inventor)

[edited by - Fruny on January 23, 2004 2:26:39 AM]