CODEWITHSUNDEEP
c++vectormemory-managementheap-memorydynamic-memory-allocation
Gromulus-Romulus
Gromulus-Romulus

Reputation: 165

C++ : How to safely deallocate a heap-allocated array of vectors?

I am currently working with code that at the moment requires me to make an array of vectors (I am new to C++ - if this is an absolutely terrible idea, I would greatly appreciate the feedback).

Let's say I allocate memory on the heap for my vectors like so:

#include <iostream>
#include <vector>
#include <random>

int main() {
    typedef std::vector<double> doubleVec;
    long N = 1000;
    long M = 1000;

    doubleVec *array = new doubleVec[N];

    for (long i = 0; i < N; i++) {
        doubleVec currentVec = array[i];
        currentVec.resize(M);
        for (long j = 0; j < M; j++)
            currentVec[j] = std::rand();
    }
    // ... do something with the data structure

   delete [] array;
}

When I've done everything I need to do with the data, how should I safely deallocate this data structure?

NOTE: There were other things I did wrong in my inital post that I didn't intend to be the focus of the discussion (uninitialized variables, didn't resize vectors, etc). I fixed those now. Thank you all for pointing those out.

Upvotes: 1

Views: 225

Answers (2)

Dmitry Kuzminov
Dmitry Kuzminov

Reputation: 6584

The problem is not in deallocating but in each vector allocation. Where in your code do you use the M value (except while accessing the elements)? There are other problems in your code, so the quick fix is:

    for (long i; i < N; i++) {
        doubleVec &currentVec = array[i];
        currentVec.resize(M);
        for (long j; j < M; j++)
            currentVec[j] = std::rand();
    }

Pay special attention that currentVec is a reference: otherwise no changes would be stored in the array.

Anyway, the main question everybody would have is: why do you need to have an array of vectors?.. The vector of vectors is a much more elegant solution.

Update: I've missed the fact that you have forgotten to initialize both i and j. In addition to the advice to initialize them I would recommend to use the auto keyword that would make it impossible to leave the variable uninitialized:

    for (auto i=0UL; i < N; i++) {
        doubleVec &currentVec = array[i];
        currentVec.resize(M);
        for (auto j=0UL; j < M; j++)
            currentVec[j] = std::rand();
    }

0UL means zero of the type unsigned long.

Upvotes: 1

eerorika
eerorika

Reputation: 238351

f this is an absolutely terrible idea, I would greatly appreciate the feedback).

Yes, this is a terribly bad idea. To be specific, owning bare pointers are a bad idea. Instead of manually allocating a dynamic array, it is usually better to use a container such as std::vector.

How to safely deallocate a heap-allocated array of vectors?

By using a vector instead of manual dynamic array. In this case, a simple solution is to use a vector of vectors.

A potentially better solution would be to allocate a single flat vector of doubles of size 1000*1000 where elements of each "subvector" is after another. This requires a bit of simple math to calculate the index of the sub vectors, but is in most use cases faster.

Other notes:

typedef std::vector<double> doubleVec;

Avoid obfuscating the program by hiding type names like this.

 for (long j; j < M; j++)
      ^^^^^^

You leave this variable uninitialised. When the indeterminate value is used later, the behaviour of the program is undefined.

Furthermore, you forgot to include the standard headers which define std::vector and std::rand.

I got a seg fault

See the other answer regarding you not actually adding any elements to the vectors that are in the array. This, and the uninitialised variables are the most likely reason for your segfault depending on what "do something" does.

Upvotes: 1

Related Questions