473,327 Members | 2,081 Online
Bytes | Software Development & Data Engineering Community
Post Job

Home Posts Topics Members FAQ

Join Bytes to post your question to a community of 473,327 software developers and data experts.

the correct way to delete a map

Hi,

I have a map containing pointers. When I destroy the map I want to
delete
all the pointers.

typedef std::map<std::string, const T*Table;

void destroy_map ()
{
for (Table::iterator i = table_.begin(); i != table_.end(); ++i)
{
delete (*i).second;
table_.erase (i);
}
}
this crashes. Does the erase() invalidate the iterator?

This was the code I found on the net (latout fixed a bit)

for (map::iterator itr = myMap.begin(); itr != myMap.end())
{
if(itr->value == something)
myMap.erase(itr++);
else
itr++;
}

which it is claimed is from Josuttis. I suppose the fact that it
didn't compile should of made me doubt this...
--
Nick Keighley

Nov 8 '07 #1
17 50319
On 8 Nov, 10:55, Nick Keighley <nick_keighley_nos...@hotmail.com>
wrote:
Hi,

I have a map containing pointers. When I destroy the map I want to
delete
all the pointers.

typedef std::map<std::string, const T*Table;

void destroy_map ()
{
for (Table::iterator i = table_.begin(); i != table_.end(); ++i)
{
delete (*i).second;
table_.erase (i);
}

}

this crashes. Does the erase() invalidate the iterator?

This was the code I found on the net (latout fixed a bit)

for (map::iterator itr = myMap.begin(); itr != myMap.end())
{
if(itr->value == something)
myMap.erase(itr++);
else
itr++;

}

which it is claimed is from Josuttis. I suppose the fact that it
didn't compile should of made me doubt this...
ah! more poking around on the net. erase() does invalidate the
iterator. That's why the net code does a post increment in the erase
call.
So preumably I want:-

void destroy_map ()
{
for (Table::iterator i = table_.begin(); i != table_.end();)
{
delete (*i).second;
table_.erase (i++);
}
}

since this code is actually in a DTOR and table_ is member variable
would I be better leaving the destruction of the map to the DTOR?

class Symbol_table
{
public:
Table table_;
~Symbol_table();
};

Symbol_table::~Symbol_table()
{
for (Table::iterator i = table_.begin(); i != table_.end(); ++i)
delete (*i).second;
}

should the for-loop be changed into an algorithm? for_each()?
--
Nick Keighley

Nov 8 '07 #2
Nick Keighley wrote:
table_.erase (i++);
No, the standard way is: i = table_.erase(i);

Besides, there's no need to individually erase the map elements. Just
free the memory pointed by the elements and then just clear() the map,
given that you are cleaning it completely.
Nov 8 '07 #3
Nick Keighley <ni******************@hotmail.comwrote:
I have a map containing pointers. When I destroy the map I want to
delete all the pointers.

typedef std::map<std::string, const T*Table;

void destroy_map ()
{
for (Table::iterator i = table_.begin(); i != table_.end(); ++i)
{
delete (*i).second;
table_.erase (i);
}
}
this crashes. Does the erase() invalidate the iterator?
Yes. Try this instead:

void destroy_map()
{
for ( Table::iterator i = table_.begin(); i != table_.end(); ++i )
{
delete i->second;
i->second = 0; // I don't think this is strictly necessary...
}
table_.clear();
}

or if you want to have fun with algorithms:

typedef map<int, int*Table;

template < typename T >
void delete_second( T& t ) {
delete t.second;
t.second = 0;
}

void destroy_map()
{
for_each( table_.begin(), table_.end(),
&delete_second<Table::value_type);
table_.clear();
}
Nov 8 '07 #4
Nick Keighley wrote:
On 8 Nov, 10:55, Nick Keighley <nick_keighley_nos...@hotmail.com>
wrote:
>Hi,

I have a map containing pointers. When I destroy the map I want to
delete
all the pointers.

typedef std::map<std::string, const T*Table;

void destroy_map ()
{
for (Table::iterator i = table_.begin(); i != table_.end(); ++i)
{
delete (*i).second;
table_.erase (i);
}

}

this crashes. Does the erase() invalidate the iterator?

This was the code I found on the net (latout fixed a bit)

for (map::iterator itr = myMap.begin(); itr != myMap.end())
{
if(itr->value == something)
myMap.erase(itr++);
else
itr++;

}

which it is claimed is from Josuttis. I suppose the fact that it
didn't compile should of made me doubt this...

ah! more poking around on the net. erase() does invalidate the
iterator. That's why the net code does a post increment in the erase
call.
So preumably I want:-

void destroy_map ()
{
for (Table::iterator i = table_.begin(); i != table_.end();)
{
delete (*i).second;
table_.erase (i++);
}
}

since this code is actually in a DTOR and table_ is member variable
would I be better leaving the destruction of the map to the DTOR?
It definitely would. And I'll tell you more, that if you use a smart
pointer you can also forget about the deallocation. The bad thing is
that you need to use additional libraries, such as boost.

Then you can have something as:

#include <map>
#include <boost/shared_ptr.h>

typedef boost::shared_ptr<intElementPtr;
typedef std::map<int, ElementPtrTable;

int main(){
Table t;
t[1] = ElementPtr(new int(1));
t[2] = ElementPtr(new int(2));
}

this will be correctly deleted by the map destructor.

Regards,

Giuseppe


Nov 8 '07 #5
Nick Keighley <ni******************@hotmail.comwrote:
ah! more poking around on the net. erase() does invalidate the
iterator. That's why the net code does a post increment in the erase
call.
So preumably I want:-

void destroy_map ()
{
for (Table::iterator i = table_.begin(); i != table_.end();)
{
delete (*i).second;
table_.erase (i++);
}
}

since this code is actually in a DTOR and table_ is member variable
would I be better leaving the destruction of the map to the DTOR?

class Symbol_table
{
public:
Table table_;
~Symbol_table();
};

Symbol_table::~Symbol_table()
{
for (Table::iterator i = table_.begin(); i != table_.end(); ++i)
delete (*i).second;
}

should the for-loop be changed into an algorithm? for_each()?
Let the destructor take care of the map, unless you find yourself
putting this kind of loop in all over the place.

I showed in another post how to switch it to using for_each, but I'm not
sure of the utility. I can't think off-hand of any other uses for the
functor, so the only gain would be the removal of the loop.

If, however, you find you have to go through maps like this more than
once in your program, then you should probably write a separate function
for it.

template < typename Map >
void delete_map_ptrs( Map& m ) {
for ( Map::iterator it = m.begin(); it != m.end(); ++it )
{
delete it->second;
it->second = 0;
}
}
Nov 8 '07 #6
Juha Nieminen wrote:
Nick Keighley wrote:
> table_.erase (i++);

No, the standard way is: i = table_.erase(i);

Besides, there's no need to individually erase the map elements. Just
free the memory pointed by the elements and then just clear() the map,
given that you are cleaning it completely.
Sorry, Juha, but map<>::erase returns void.
Nov 8 '07 #7
red floyd wrote:
Juha Nieminen wrote:
>Nick Keighley wrote:
>> table_.erase (i++);

No, the standard way is: i = table_.erase(i);

Besides, there's no need to individually erase the map elements.
Just free the memory pointed by the elements and then just clear()
the map, given that you are cleaning it completely.

Sorry, Juha, but map<>::erase returns void.
In C++0x it will return an iterator. Many library implementations
already have that. While in the currect Standard it's 'void', check
your implementation for it as a possible extension.

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask
Nov 8 '07 #8
On Nov 8, 2:38 pm, Juha Nieminen <nos...@thanks.invalidwrote:
Nick Keighley wrote:
table_.erase (i++);
No, the standard way is: i = table_.erase(i);
Sorry, Juha, but for some silly reason, map<>::erase() doesn't
return an iterator. (This will be corrected in the next version
of the standard, and I think some implementations already
support it, but if you want to be portable and/or standards
conformant, you can't do it.)
Besides, there's no need to individually erase the map elements. Just
free the memory pointed by the elements and then just clear() the map,
given that you are cleaning it completely.
Formally, having a pointer to a deleted object in the map is
undefined behavior. So you really have to either erase or null
the pointer in the map before deleting, something like:
T* tmp = *i ;
table.erase( i ++ ) ;
delete tmp ;
or
T* tmp = NULL ;
std::swap( tmp, *i ) ;
delete tmp ;
(And that's really in code---using swap is definitly stylish.)

Practically, of course, I wouldn't worry about it, and would do
it as you suggest. Or use smart pointers: the case where a
container is the actual owner of an object is one of the cases
where their use is justified. (I'll admit that in my own code,
in almost 20 years of C++, I've only found one case where a
container actually owned something it pointed to. But that's a
different issue.)

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34

Nov 9 '07 #9
On 9 Nov, 09:38, James Kanze <james.ka...@gmail.comwrote:

<snip>
Formally, having a pointer to a deleted object in the map is
undefined behavior. So you really have to either erase or null
the pointer in the map before deleting
<snip>
Practically, of course, I wouldn't worry about it, and would do
it as you suggest. Or use smart pointers: the case where a
container is the actual owner of an object is one of the cases
where their use is justified. (I'll admit that in my own code,
in almost 20 years of C++, I've only found one case where a
container actually owned something it pointed to. But that's a
different issue.)
well obviously I can't disagree with your experience. But it seemed
not unusual for a container to own an object it pointed to. If you're
newing a lot of similar objects then you need to keep them somewhere.
Why not a container?

The code that prompted my original query was a symbol table.
A definition file is parsed to build the symbol table

typedef std::map<std::string,const Type*Table;

This symbol table is then used to decode a stream of data.
The first octet read is looked up to yield the message name,
which is then looked up to find the Type. Type knows how
to parse the rest of the stream.

Other cases might be Calls in a telecommunication system.
Drawable items in a drawing program etc.

I'd have thought a container that owned things it pointed to was
very common!
--
Nick Keighley

Nov 9 '07 #10
Nick Keighley <ni******************@hotmail.comwrote:
it seemed not unusual for a container to own an object it pointed
to. If you're newing a lot of similar objects then you need to keep
them somewhere. Why not a container?
There is some confusion here I think. When you say "you're newing" you
don't actually mean the programmer, rather some object is instantiating
some other object. Generally, the instantiating object is the owner.
Nov 9 '07 #11
On 9 Nov, 11:07, "Daniel T." <danie...@earthlink.netwrote:
Nick Keighley <nick_keighley_nos...@hotmail.comwrote:
it seemed not unusual for a container to own an object it pointed
to. If you're newing a lot of similar objects then you need to keep
them somewhere. Why not a container?

There is some confusion here I think. When you say "you're newing" you
don't actually mean the programmer, rather some object is instantiating
some other object. Generally, the instantiating object is the owner.
yes I meant an instantiating object was newing the Type objects.
Type is actually an abstract base class. The instantiator generates
many objects of various classes derived from Type. I can't see how
the parser *can* hold many Type objects without using a container of
some sort.

Simplifying outrageously:-

void parse_file (Token_stream& toks)
{
while (toks.more)
{
Type* type;
string name = toks.current_token;

toks.next();

// this is hidden in a factory
if (toks.current_token == "type_a")
type = new Type_a(toks);
else
if (toks.current_token == "type_b")
type = new Type_b(toks);
else
...

symbol_table.inert(name, type);
}
}

In this case the parser which built the symbol table
ceases to exist once the table is built. So it can't
own the Type objects!
this seems such a natural idiom to me and falls naturally out
of the requirement. So I'm either abusing OOP/C++ or I don't
understand
what "ownership" means.

help!
--
Nick Keighley

Nov 9 '07 #12
Nick Keighley <ni******************@hotmail.comwrote:
"Daniel T." <danie...@earthlink.netwrote:
Nick Keighley <nick_keighley_nos...@hotmail.comwrote:
it seemed not unusual for a container to own an object it
pointed to. If you're newing a lot of similar objects then you
need to keep them somewhere. Why not a container?
There is some confusion here I think. When you say "you're
newing" you don't actually mean the programmer, rather some
object is instantiating some other object. Generally, the
instantiating object is the owner.

yes I meant an instantiating object was newing the Type objects.
Type is actually an abstract base class. The instantiator generates
many objects of various classes derived from Type. I can't see how
the parser *can* hold many Type objects without using a container
of some sort.

In this case the parser which built the symbol table ceases to
exist once the table is built. So it can't own the Type objects!
So the object that instantiates the parser takes ownership of the Type
objects. The point is though that the container didn't create the
objects, so it doesn't own them. The object that created the parser and
used it as a surrogate, indirectly created the Type objects and therefor
owns them.
Nov 9 '07 #13
On Nov 9, 12:07 pm, "Daniel T." <danie...@earthlink.netwrote:
Nick Keighley <nick_keighley_nos...@hotmail.comwrote:
it seemed not unusual for a container to own an object it
pointed to. If you're newing a lot of similar objects then
you need to keep them somewhere. Why not a container?
There is some confusion here I think. When you say "you're
newing" you don't actually mean the programmer, rather some
object is instantiating some other object. Generally, the
instantiating object is the owner.
Generally, in an OO design (which works well in many fields of
endevor), once created, an object is on its own. The object
itself decides when it's no longer needed, and deletes itself.

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34

Nov 9 '07 #14
On Thu, 08 Nov 2007 02:55:35 -0800, Nick Keighley wrote:
>I have a map containing pointers. When I destroy the map I want to
delete all the pointers.

typedef std::map<std::string, const T*Table;
Std containers were designed only for values. They don't work with
pointers (to objects), at least not without clumsy and prohibitive
workarounds. This is one of the major reasons for the low acceptance
of STL in the real world (in contrast to the ongoing STL hype among a
small group of C++ gurus and "gurus").
So, better look for a container that works with pointers. You can even
built your own on top of an STL container.
BTW, std::string as key can introduce problems, too. Depending on your
basic_string implementation it may dynamically allocate an internal
buffer for each copy. This may be unacceptable for a real parser.
--
Roland Pibinger
"The best software is simple, elegant, and full of drama" - Grady Booch
Nov 10 '07 #15
On Fri, 09 Nov 2007 18:36:32 -0000, James Kanze wrote:
>Generally, in an OO design (which works well in many fields of
endevor), once created, an object is on its own. The object
itself decides when it's no longer needed, and deletes itself.
But not in C++. Actually, a dynamically created object cannot decide
on its own when it's no longer needed.
The classic C++ idiom is to let an 'owner object' manage dynamically
created objects, preferably by deleting them in its destructor, a.k.a.
RAII. The best working idiom in this context is 'Creator As Sole
Owner' http://www.ddj.com/184409895 .
--
Roland Pibinger
"The best software is simple, elegant, and full of drama" - Grady Booch
Nov 10 '07 #16
On Thu, 08 Nov 2007 02:55:35 -0800, Nick Keighley wrote:
for (map::iterator itr = myMap.begin(); itr != myMap.end())
{
if(itr->value == something)
myMap.erase(itr++);
else
itr++;
}
for(map::iterator j, i = myMap.begin(); i != myMap.end(); i = j)
{
j = i; ++j;

if(i->value == something)
myMap.erase(i);
}

This is how I do it as I wasn't aware of erase() returning an
iterator (which apparently may be the case in some implementations).

If you want to avoid the redundant default constructor
call of j, then write it as follows:

for(map::iterator i = myMap.begin(); i != myMap.end(); )
{
map::iterator j = i; ++j;

if(i->value == something) myMap.erase(i);

i=j;
}

However, this code is more prone for errors: for example,
if you use "continue;", it will omit "i=j;", causing an
infinite loop at worst.

--
Joel Yliluoma - http://bisqwit.iki.fi/
: comprehension = 1 / (2 ^ precision)
Nov 12 '07 #17
On Thu, 08 Nov 2007 02:55:35 -0800, Nick Keighley wrote:
for (map::iterator itr = myMap.begin(); itr != myMap.end())
{
if(itr->value == something)
myMap.erase(itr++);
else
itr++;
}
for(map::iterator j, i = myMap.begin(); i != myMap.end(); i = j)
{
j = i; ++j;

if(i->value == something)
myMap.erase(i);
}

This is how I do it as I wasn't aware of erase() returning an
iterator (which apparently may be the case in some implementations).

If you want to avoid the redundant default constructor
call of j, then write it as follows:

for(map::iterator i = myMap.begin(), j = i; i != myMap.end(); i = j)
{
++j;
if(i->value == something) myMap.erase(i);
}

--
Joel Yliluoma - http://bisqwit.iki.fi/
: comprehension = 1 / (2 ^ precision)
Nov 12 '07 #18

This thread has been closed and replies have been disabled. Please start a new discussion.

Similar topics

2
by: Ian McBride | last post by:
(was: delete() confusion) I have a class with multiple base classes. One of these base classes (base1) has its own new/delete operators and nothing else. Another base class (base 2) has a...
8
by: Randy Gordon | last post by:
Say I do the following: char **names = new char*; for (int i = 0; i < 100; i++) { names = new char; } When I'm done with the array of character pointers, how should I delete it? Like this:
1
by: Nimmi Srivastav | last post by:
There's a rather nondescript book called "Using Borland C++" by Lee and Mark Atkinson (Que Corporation) which presents an excellent discussion of overloaded new and delete operators. In fact there...
3
by: Nimmi Srivastav | last post by:
There's a rather nondescript book called "Using Borland C++" by Lee and Mark Atkinson (Que Corporation) which presents an excellent discussion of overloaded new and delete operators. I am...
3
by: Christian Meier | last post by:
Hi NG I don't know if this problem is system dependend but I don't think so. So, here is my assumption: "delete" deletes a variable. "delete" deletes an array. Is this correct so far? And now...
4
by: Stephen | last post by:
Hello People, Using MS Access 2003 VBA I get the error 3020 Update or CancelUpdate without AddNew or Edit when I run through the following code. Can anyone help suggest anything to try? Thanks....
10
by: junw2000 | last post by:
Hi, Below is a small code about memory allocation and deallocation. #include <cstdlib> #include <iostream> using namespace std; class X { public: void* operator new(size_t sz) throw (const...
5
by: mkaushik | last post by:
Hi everyone, Im just starting out with C++, and am curious to know how "delete <pointer>", knows about the number of memory locations to free. I read somewhere that delete frees up space...
1
by: Gonçalo Rodrigues | last post by:
Hi all, I am a little confused about the delete operator, so I have a question. Suppose we have something like class Base { public: void* operator new(std::size_t size); void operator...
1
by: rasmidas | last post by:
I have a similar kind of code as below. But I could not guess how to delete. Please help me. #include<stdio.h> #include<string.h> #include<stdlib.h> int main() { char* items; ...
0
by: DolphinDB | last post by:
Tired of spending countless mintues downsampling your data? Look no further! In this article, you’ll learn how to efficiently downsample 6.48 billion high-frequency records to 61 million...
0
by: ryjfgjl | last post by:
ExcelToDatabase: batch import excel into database automatically...
0
isladogs
by: isladogs | last post by:
The next Access Europe meeting will be on Wednesday 6 Mar 2024 starting at 18:00 UK time (6PM UTC) and finishing at about 19:15 (7.15PM). In this month's session, we are pleased to welcome back...
1
isladogs
by: isladogs | last post by:
The next Access Europe meeting will be on Wednesday 6 Mar 2024 starting at 18:00 UK time (6PM UTC) and finishing at about 19:15 (7.15PM). In this month's session, we are pleased to welcome back...
0
by: Vimpel783 | last post by:
Hello! Guys, I found this code on the Internet, but I need to modify it a little. It works well, the problem is this: Data is sent from only one cell, in this case B5, but it is necessary that data...
0
by: jfyes | last post by:
As a hardware engineer, after seeing that CEIWEI recently released a new tool for Modbus RTU Over TCP/UDP filtering and monitoring, I actively went to its official website to take a look. It turned...
0
by: ArrayDB | last post by:
The error message I've encountered is; ERROR:root:Error generating model response: exception: access violation writing 0x0000000000005140, which seems to be indicative of an access violation...
1
by: PapaRatzi | last post by:
Hello, I am teaching myself MS Access forms design and Visual Basic. I've created a table to capture a list of Top 30 singles and forms to capture new entries. The final step is a form (unbound)...
1
by: CloudSolutions | last post by:
Introduction: For many beginners and individual users, requiring a credit card and email registration may pose a barrier when starting to use cloud servers. However, some cloud server providers now...

By using Bytes.com and it's services, you agree to our Privacy Policy and Terms of Use.

To disable or enable advertisements and analytics tracking please visit the manage ads & tracking page.