delete pointers in destructor

Discussion in 'C++' started by TBass, Dec 19, 2007.

  1. TBass

    TBass Guest

    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
    TBass, Dec 19, 2007
    #1
    1. Advertising

  2. TBass wrote:
    > 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
    --
    Please remove capital 'A's when replying by e-mail
    I do not respond to top-posted replies, please don't ask
    Victor Bazarov, Dec 19, 2007
    #2
    1. Advertising

  3. TBass

    TBass Guest

    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
    TBass, Dec 19, 2007
    #3
  4. TBass wrote:
    > 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
    --
    Please remove capital 'A's when replying by e-mail
    I do not respond to top-posted replies, please don't ask
    Victor Bazarov, Dec 19, 2007
    #4
  5. TBass

    David Harmon Guest

    On Wed, 19 Dec 2007 07:33:38 -0800 (PST) in comp.lang.c++, TBass
    <> wrote,
    >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.
    David Harmon, Dec 19, 2007
    #5
  6. TBass

    TBass Guest

    > 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.


    > > 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.
    >


    Great. Thanks for the tips.

    Very much appreciated,
    T
    TBass, Dec 19, 2007
    #6
  7. On 2007-12-19 17:32, TBass wrote:
    >> 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.


    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.

    --
    Erik Wikström
    Erik Wikström, Dec 19, 2007
    #7
  8. TBass

    Andre Kostur Guest

    TBass <> wrote in news:7712834f-c002-421c-9189-
    :

    > 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.
    Andre Kostur, Dec 19, 2007
    #8
  9. 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
    Yakov Gerlovin, Dec 20, 2007
    #9
  10. TBass

    James Kanze Guest

    On Dec 19, 6:53 pm, Erik Wikström <> wrote:
    > On 2007-12-19 17:32, TBass wrote:


    > >> 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.


    > 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).

    --
    James Kanze (GABI Software) email:
    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
    James Kanze, Dec 20, 2007
    #10
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Xamalek
    Replies:
    7
    Views:
    681
  2. frs
    Replies:
    20
    Views:
    736
    Alf P. Steinbach
    Sep 21, 2005
  3. arun
    Replies:
    2
    Views:
    534
    benben
    Jun 13, 2006
  4. Jimmy Hartzell
    Replies:
    0
    Views:
    411
    Jimmy Hartzell
    May 19, 2008
  5. Jimmy Hartzell
    Replies:
    2
    Views:
    1,163
    Jimmy Hartzell
    May 20, 2008
Loading...

Share This Page