delete pointers in destructor

T

TBass

Hi,

Switching some code from C to C++, and I have a destructor question:

In the C code, when the program wants to close and free all memory, I
have them call a Close function. The linked-lists get free'd.

In the C++ versions, my class has a linked list items of another
class, which also has a linked list of another class. I was going to
add a Close function to the classes, but is the C++ way to just put
the delete statements in the destructor and let it get handled
automatically?

Thanks in advance,
T
 
V

Victor Bazarov

TBass said:
Switching some code from C to C++, and I have a destructor question:

In the C code, when the program wants to close and free all memory, I
have them call a Close function. The linked-lists get free'd.

In the C++ versions, my class has a linked list items of another
class, which also has a linked list of another class. I was going to
add a Close function to the classes, but is the C++ way to just put
the delete statements in the destructor and let it get handled
automatically?

Usually, yes. There are exceptions, of course. If you actually use
'std::list' as your member, you don't need to delete anything, the
list will take care of that. The exceptions to that rule is if your
list is of pointers to dynamically created objects. The rule is, if
you obtained the pointer through 'new' (or 'new[]'), you must dispose
of it using 'delete' (or 'delete[]'). The exception to that rule, in
turn, is if you're using some kind of "smart" pointer type to hold
the naked pointer for you.

Perhaps if you're most specific in your question, you'd get a more
specific answer :)

V
 
T

TBass

Sorry. I didn't mean to be vague.

[snip]
The exceptions to that rule is if your list is of pointers to
dynamically created objects.
[/snip]

I am using std::list, but it is a list of pointers to an instance of
the class, which I created with the new statement.

Here's a quick outline of what I'm doing:

class tag
{
...
}

class device
{
...
std::list<tag *> m_listTags
}


device::AddTag
{
tag *mytag = new tag;
m_listTags.push_back( mytag );
}


class project
{
...
std::list<device *> m_listDevices
}

project::AddDevice
{
device *mydevice = new device;
m_listDevices.push_back( device );
}


So I'm thinking I should just add to the destructors:

device::~device(void)
{
std::list<tag *>::iterator item;
device *mytag;
for ( item = m_listTags.begin(); item != m_listTags.end(); +
+item )
{
mytag = *item;
delete mytag
}

m_listTags.empty()
}

project::~project(void)
{
std::list<device *>::iterator item;
device *mydevice;
for ( item = m_listDevices.begin(); item != m_listDevices.end(); +
+item )
{
mydevice = *item;
delete mydevice
}

m_listDevices.empty()
}

Thanks!
T
 
V

Victor Bazarov

TBass said:
Sorry. I didn't mean to be vague.

[snip]
The exceptions to that rule is if your list is of pointers to
dynamically created objects.
[/snip]

I am using std::list, but it is a list of pointers to an instance of
the class, which I created with the new statement.

Do you really need to? What if you keep a list of tags or list of
devices instead? 'std::list' creates its elements dynamically
anyway. Let your lists manage memory.

If you _have to_ keep a list of pointers, you're doing it correctly,
with a bit of excess, see below.
Here's a quick outline of what I'm doing:

class tag
{
...
}

class device
{
...
std::list<tag *> m_listTags

Why not change it to

std::list<tag> m_listTags;

?
}


device::AddTag
{
tag *mytag = new tag;
m_listTags.push_back( mytag );

In case you decide to change, you would just do

m_listTags.push_back(tag());

Isn't it better?
}


class project
{
...
std::list<device *> m_listDevices

Same here, why not just

std::list<device> m_listDevices;

? I know one possible answer, but I don't want to speculate.
}

project::AddDevice
{
device *mydevice = new device;
m_listDevices.push_back( device );
}


So I'm thinking I should just add to the destructors:

Below my comments assume you still go with the lists of pointers.
device::~device(void)
{
std::list<tag *>::iterator item;
device *mytag;
^^^^^^^^^^^^^
You don't need this.
for ( item = m_listTags.begin(); item != m_listTags.end(); +
+item )
{
mytag = *item;
^^^^^^^^^^^^^^
You don't need this either.
delete mytag

Change it to

delete *item;
}

m_listTags.empty()


No need to empty, it's about to be destructed.
}

project::~project(void)
{
std::list<device *>::iterator item;
device *mydevice;

Same thing. No need for this local var.
for ( item = m_listDevices.begin(); item != m_listDevices.end(); +
+item )
{
mydevice = *item;
delete mydevice

Just do

delete *item;

instead.
}

m_listDevices.empty()

No need to empty, it's about to be destructed.
}

Thanks!
T

V
 
D

David Harmon

On Wed, 19 Dec 2007 07:33:38 -0800 (PST) in comp.lang.c++, TBass
I am using std::list, but it is a list of pointers to an instance of
the class, which I created with the new statement.

You should look carefully at whether you really need to do that, or
if you can just use a list of the instances.
 
T

TBass

Do you really need to? What if you keep a list of tags or list of
devices instead? 'std::list' creates its elements dynamically
anyway. Let your lists manage memory.

Agreed, which is what I originally intended. However, I have to pass
out pointers to instances of the class to forms for display and
editing. I wasn't sure how this would affect the list, or if changes
to the list in other functions could cause an address I had previously
passed out to become invalid, so I figured I would play it safe.

^^^^^^^^^^^^^
You don't need this.


^^^^^^^^^^^^^^
You don't need this either.


Change it to

delete *item;



No need to empty, it's about to be destructed.

Great. Thanks for the tips.

Very much appreciated,
T
 
E

Erik Wikström

Agreed, which is what I originally intended. However, I have to pass
out pointers to instances of the class to forms for display and
editing. I wasn't sure how this would affect the list, or if changes
to the list in other functions could cause an address I had previously
passed out to become invalid, so I figured I would play it safe.

The list is node-based, a pointer to an element will be valid as long as
you do not delete that element, regardless of what you do with the rest
of the elements in the list. This is not true for all containers though.
 
A

Andre Kostur

Hi,

Switching some code from C to C++, and I have a destructor question:

In the C code, when the program wants to close and free all memory, I
have them call a Close function. The linked-lists get free'd.

In the C++ versions, my class has a linked list items of another
class, which also has a linked list of another class. I was going to
add a Close function to the classes, but is the C++ way to just put
the delete statements in the destructor and let it get handled
automatically?

If I understand correctly, yes. Your class initiates the deletion of your
linked list (which I'd hope is in some appropriate class itself, like
std::list<>). Your linked list will know how to dispose of itself. Your
"other object" would know to dispose of the linked list it contains.
 
Y

Yakov Gerlovin

Indeed, generally, it is better to store the objects, rather than the
pointer to the object in the container.
However, in some cases this is not the solution. For example, when you
want to store instances of different classes with common base class.
Another example ( although very rare ) is when copying the object
( i.e. copy constructor ) is very expensive in means of time or
allocated resources.
In those cases, it could be useful to store smart pointers in your
container.
This way you get the advantage of using pointers, without the need to
delete the instances in the container.

My 2 cents
 
J

James Kanze

On 2007-12-19 17:32, TBass wrote:
The list is node-based, a pointer to an element will be valid
as long as you do not delete that element, regardless of what
you do with the rest of the elements in the list. This is not
true for all containers though.

He does have to support copy on the objects, however. If the
objects have identity, then they usually don't support copy or
assignment. And if they don't have identity, he probably should
be returning them by value anyway, rather than by pointers (but
there are exceptions to both cases).
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,743
Messages
2,569,478
Members
44,898
Latest member
BlairH7607

Latest Threads

Top