Proper initialization and destruction of a C++ array of object pointers?

Question

I want to create an array of pointers to custom objects Image, but I am not sure if I am doing it properly (and I dont have any experience with arrays of pointers). I have a constructor that takes in one image as the first element and the array size, but I don't think I have properly created an array of image pointers. I know it is probably a lot easier, but I do not want to use vectors.

In the header file, I have:

class Album {
public:
  unsigned arrmax;
  Image** imgar;
  Image basepic;

And in the cpp file I have a constructor:

Album::Album(const Image & picture, unsigned max) {
arrmax = max;
basepic = picture; //operator overloaded
imgar = new Image*[arrmax]; //array of Image pointers 
for (unsigned i = 0; i < max; i++) {
   imgar[i] = NULL;
  }
 imgar[0] = &basepic;
}

My destructor looks like this:

Album::~Album() {
if (imgar != NULL) {
for (unsigned i = 0; i < this->arrmax; i++) {
  if (imgar[i] != NULL) {
    delete imgar[i]; // delete[] or delete??
   } 
  }
 }
}

For the destructor, would I also have to do delete[] imgar as well after iterating through the elements? Or am I just not deleting the right things?

You probably want to use a `std::vector>>` and forget about the manual memory management at all. — πάντα ῥεῖ, Sep 25 '18 at 20:12
`delete` what you `new` and `delete[]` what you `new[]`. If you neither `new` nor `new[]`, you don't `delete` nor `delete[]`. `imgar[i]` is a pointer to whatever `basepic` is, you don't seem to `new` it. Avoid these kinds of concerns and use smart pointers like `std::unique_ptr` or standard containers. — François Andrieux, Sep 25 '18 at 20:17
Sorry, one indirection too much, `std::vector>` would be enough. — πάντα ῥεῖ, Sep 25 '18 at 20:19
More than likely what you really need is just a `std::vector` — NathanOliver, Sep 25 '18 at 20:20
modern c++ programs should never normally use naked pointers, use shared_ptr or unique_ptr. — pm100, Sep 25 '18 at 20:38
To those who close for duplicated of "How do I declare a 2d array in C++ using new?". Please, read the question: Is not a 2d array; it does not ask for `new`; Is not the same question, the answers are not related neither. If you don't like the question, find another excuse. — Adrian Maire, Sep 25 '18 at 21:03

score 0 · Answer 1 · answered Sep 25 '18 at 20:42

0

You do not need the for loop in the destructor because you want to delete a pointer array created using new []. This alone is sufficient:

Album::~Album() {
    delete[] imgar;
}

answered Sep 25 '18 at 20:42

Some Guy

1,787
11
15

Adrian Maire · Answer 2 · 2018-09-25T20:55:45.553

From your question, I understand you prefer some direction/good-practices rather than a direct answer.

Some good practices:

Try to avoid direct memory management, it is error prone. Whenever you can, prefer smart pointers (unique, shared, weak)
You may use std::array/std::vector as an improved array, it manage the size and has some other good to have features.
For array sizes, prefer size_t over "unsigned int"
Most of the time, the container range loop is more readable and faster.

So your code could be something like this:

class Album{
public:
    std::vector<std::unique_ptr<Image>> _imgVect;
    Image basepic;
    ...
};

Album::Album(const Image & picture, unsigned max) {
    _imgVect.reserve(max);
    basepic = picture; //operator overloaded
    // default of unique_ptr is already nullptr

    // Not really a good practice, but without more information
    // of what you are trying, it is the best I could imagine.
    _imgVect[0] = std::unique_ptr<Image>(&basepic, [](Image*){});
}

// Use default destructor

but I do not want to use vectors

Any reason in particular? If you do not want containers, yes: new/delete is your choice. You might still use the smart pointer.

For the destructor, would I also have to do delete[] imgar as well after iterating through the elements? Or am I just not deleting the right things?

using delete[] will delete the array, but not the content which is saved for each pointer in the array. You must use delete over each element before to delete all the array with delete[].

You have to be very careful with what you delete, for example, the first element of your array (in your question code) is not a heap allocated object, so trying to delete it will have undefined behaviour (usually a crash).

Proper initialization and destruction of a C++ array of object pointers?

2 Answers2