CODEWITHSUNDEEP
c++arraysmemory-managementstructnew-operator
Daniel Sloof
Daniel Sloof

Reputation: 12706

Array of structs and new / delete

I have a struct like this:

class Items 
{
private:
    struct item
    {
        unsigned int a, b, c;
    };
    item* items[MAX_ITEMS];
}

Say I wanted to 'delete' an item, like so:

items[5] = NULL;

And I created a new item on that same spot later:

items[5] = new item;

Would I still need to call delete[] to clean this up? Or won't this be needed since bounds of array items[] are known before compiling?

Is setting that pointer to NULL valid or should I be calling delete there?

Upvotes: 7

Views: 24088

Answers (8)

Max Lybbert
Max Lybbert

Reputation: 20047

There are a few, related, questions here:

  1. According to the code you posted, the array itself is not allocated on the heap unless the struct is, so you don't need to delete[] the array. If you created the array with new[] you would have to delete[] it.
  2. The code posted doesn't say how the objects being pointed to from the array are allocated. If you allocate those objects on the stack you must not delete them (then again, this is highly unlikely because your pointers will become invalid when the objects they point to fall out of scope). If you allocated them on the heap (with new) then you must delete them when they fall out of scope.
  3. As others have already suggested, life is much easier if you use a container -- especially an STL container -- and smart pointers -- which for now means pointers out of Boost.

Upvotes: 1

j_random_hacker
j_random_hacker

Reputation: 51326

Just to be clear: you refer to calling "delete[]". I think you mean delete.

I mention this because C++ has two separate operators, operator delete and operator delete[]. The latter is used for deleting arrays of objects allocated with operator new[], and does not apply in this case. You have an array of pointers to objects, which you must have initialised with repeated calls to operator new rather than a single call to operator new[].

All I'm really trying to say is: your use of delete[] is confusing and ambiguous; change it to delete.

Upvotes: 1

Blaisorblade
Blaisorblade

Reputation: 6498

Say I wanted to 'delete' an item, like so:

items[5] = NULL;

I know little Visual Basic, but that smells like a Visual Basic programming idiom, since "Set a = None" (or Null, I'm not sure) would delete the object pointed by a (or rather decrement its reference count, for COM objects).

As somebody else noted, you should use either:

delete items[5];
items[5] = newContent;

or:

delete items[5];
items[5] = NULL;

After delete[5], the only possible use of the pointer stored in items[5] is causing you trouble. What's worse is that it might happen to work at the beginning, and start failing only when you allocate something else over the space previously used by *items[5]. Those are the causes which make C/C++ programming "interesting", i.e. really annoying (even for who likes C like me).

Writing just delete items[5]; saves what can be an useless write, but that's a premature optimization.

Upvotes: 1

Pyry Jahkola
Pyry Jahkola

Reputation: 633

As Kluge pointed out, you'd leak the object at index 5 like that. But this one really sounds like you shouldn't do this manually but use a container class inside Item. If you don't actually need to store these item objects as pointers, use std::vector<item> instead of that array of MAX_ITEMS pointers. You can always insert or erase vector elements in the middle as well if you need to.

In case you need to store the objects as pointers (usually if struct item is actually polymorphic, unlike in your example), you can use boost::ptr_vector<item> from Boost.PtrContainer instead.

Example:

class Items {
private:
    struct item {
        unsigned int a, b, c;
    };
    std::vector<item> items;
}

if (items.size() > 5) // (just to ensure there is an element at that position)
    items.erase(items.begin() + 5); // no need to use operator delete at all

Upvotes: 6

Joao Vilaca
Joao Vilaca

Reputation: 638

To delete an item use:

delete items[5];

after deleting the item it is advisable to set the deleted pointer to NULL, so you won't have an error if you later delete it again by mistake.

items[5] = NULL

Upvotes: 1

Paige Ruten
Paige Ruten

Reputation: 176803

You need to call delete before setting it to NULL. (Setting it to NULL isn't required, it just helps reduce bugs if you accidentally try to dereference the pointer after deleting it.)

Remember that every time you use new, you will need to use delete later on the same pointer. Never use one without the other.

Also, new [] and delete [] go together in the same way, but you should never mix new [] with delete or new with delete []. In your example, since you created the object with new (rather than new [] which would create an array of objects) you must delete the object with delete (rather than delete []).

Upvotes: 16

Kluge
Kluge

Reputation: 3727

Setting items[5] to NULL doesn't delete the memory associated with the item, it simply sets the pointer to that item to NULL, therefore the memory is leaked.

You can delete the item by calling:

delete items[5];

Since C++ has not automatic garbage collection, you need to delete any memory you no longer need.

Upvotes: 0

Hank Gay
Hank Gay

Reputation: 72039

C++ isn't my strong suit, but I'm pretty sure you'd be leaking the memory if you set the pointer to NULL.

EDIT: The memory being leaked would be the memory being pointed to by the pointer in the array.

Upvotes: 0

Related Questions