zzari
zzari

Reputation: 313

How to delete pointers of vector

I want to delete properly all the elements of my vector. I need to fill a vector of vector of structs in the way you should see below, creating each time a new vector of struct invoking new method.

this is my code:

struct node
{
    int index;
    double value;
}

struct problem
{
    int l;
    struct node **x;
};

struct problem prob;    // set by read_problem
std::vector<node *> vettProbX;


int main(){
    node tmpValue;
    std::vector <node> *temp;

    for (int i=0; i<10; i++)
    {
        temp = new std::vector<node>;

        for (int i=0; i<10; i++)
        {
            tmpValue.index=i;
            tmpValue.value=i;
            temp->push_back(tmpValue);
        }

        vettProbX.push_back(temp->data());

    }

    prob.x=&vettProbX[0];

    //Destructor
    for(int CC=0; CC<vettProbX.size(); CC++)
    {
        delete  temp[CC];
    }
    return 0;
}

I want to delete all the new objects allocated with NEW, but i don't know how to do it correctly.

Upvotes: 0

Views: 709

Answers (3)

ciamej
ciamej

Reputation: 7068

If you don't save a pointer to the newly created vector, that you create here:

temp = new std::vector<node>;

then, there is no way to delete it and free the memory.

Change

std::vector<node *> vettProbX;

vettProbX.push_back(temp->data());

to

std::vector<std::vector<node> *> vettProbX;

vettProbX.push_back(temp);

then you can delete every item stored in vettProbX one by one

for (int i = 0; i < vettProbX.size(); i++)
    delete vettProbX[i];

if you need to access these memory segments instead of doing

vettProbX[i]

you will have to call the data method

vettProbX[i]->data()

Upvotes: 0

eerorika
eerorika

Reputation: 238351

The only object that you have allocated with new is std::vector<node> *temp;. You delete that with delete temp; Vectors own destructor will handle the destruction of its contents. But you must do that for every time that you call new. In this case you allocate inside loop so you need to call delete at the end of the loop.

But because temp is only used within scope, there is no need to allocate it dynamically. You can allocate it on the stack like this: std::vector<node> temp;. Then it will be destructed automatically when it goes out of scope (at the end of function in this case).

In your code, you allocate temp 10 times, but you leak first 9 because you don't store their pointer anywhere (And thus cannot delete them anymore). You always overwrite the pointer of previous temp. At the end of your code you try to delete contents of the last temp (which you can't because the contents are not pointers) rather than the vector itself.

To achieve what you're trying to do, what you need to do is to store all the temp pointers in a std::vector<std::vector<node>* > and in the end delete each vector inside. But you would have much nicer code if you didn't use any pointers at all.

Also, you have two nested loops that use the same loop variable. It probably won't work as you have intended.

Upvotes: 3

Steve Jessop
Steve Jessop

Reputation: 279255

I want to delete properly all the elements of my vector

It's not possible to free everything because of the strange way you allocated memory. You dynamically allocate a vector pointed to by temp, then overwrite your pointer to it in the next iteration of the loop. That's it, once your record of the pointer is gone you have no way to free it.

I need to fill a vector of vector of structs

In that case you should first create a vector<vector<node>>. You can't fill a vector of vectors if you don't have one, and your current code doesn't have one.

Upvotes: 2

Related Questions