Is it bad practice to reinitialize a pointer?

Question

I have an Image class and initially I do not know the image dimensions, so I just initialize a data_ pointer to be an array of size 0. Later when I find the image information I reinitialize data_ to a new size. Will this create any problem in memory? and is there a cleaner way to do this?

Below is the class I have written:

class Image
{
private:
    int numRows_, numCols_;
    unsigned char* data_;
public:
    Image() : numRows_(0), numCols_(0), data_(new unsigned char[0])
    {}
    void setData(int r, int c, unsigned char* data)
    {
        this->numRows_ = r;
        this->numCols_ = c;
        this->data_ = new unsigned char[r*c];
        for (int i = 0; i < r*c; i++)
        {
            this->data_[i] = data[i];
        }
    }
    int rows();
    int cols();
    unsigned char* data();
    ~Image();
};

Thanks in advance

You can't re-initialize stuff. Initialization is done only once. Anything you do after that is just assignment. — Algirdas Preidžius, Jun 01 '17 at 11:14
@EdgarRokyan What does it matter what's in the destructor? OP is leaking memory already on the call to `setData`. — Rotem, Jun 01 '17 at 11:14
In `setData`, what if `data_` is already initialized, pointing to some other data? — Some programmer dude, Jun 01 '17 at 11:14
Why create the object before you know what data to put in it? If you just create it when you know the information you can create it then and avoid problems alluded to in the other comments — doctorlove, Jun 01 '17 at 11:15
there is nothing wrong with changing the pointer's values, but you need to have a sound memory management strategy — perreal, Jun 01 '17 at 11:15
Also, the usual recommendation against using raw pointers. Prefer [`std::vector`](http://en.cppreference.com/w/cpp/container/vector), but if you *must* use pointers then use [`std::unique_ptr`](http://en.cppreference.com/w/cpp/memory/unique_ptr) instead. — Some programmer dude, Jun 01 '17 at 11:15
Just set `data_` to `nullptr` and call `delete data_` right before `new unsigned char[r*c];`. It's OK to delete `nullptr`. — Jabberwocky, Jun 01 '17 at 12:08

Rotem · Answer 1 · 2017-06-01T11:21:22.907

This will in fact leak memory. The call to new allocates memory for the array, even if it is empty. As soon as you reassign data_, the previous array is leaked and can no longer be freed.

You can either make sure you delete[] any new[] you allocate, or just don't allocate an empty array and instead set data_ to nullptr until you have meaningful data to use.

An even better idea is don't allow the creation of an object in an invalid state, require the data in the constructor - see RAII:

In RAII, holding a resource is a class invariant, and is tied to object lifetime: resource allocation (or acquisition) is done during object creation (specifically initialization), by the constructor, while resource deallocation (release) is done during object destruction (specifically finalization), by the destructor.

If you do decide to keep setData, then as mentioned in comments, you also must make sure to delete[] existing data in setData before reassigning data_, in case the method is called more than once.

Thanks @Rotem I'm going to use nullptr this time but just out of curiosity is it wise to call delete[] on private pointer `data_` using one of its public methods? I have a feeling that it should be avoided if possible. — Ijlal, Jun 01 '17 at 11:34

score 3 · Answer 2 · answered Jun 01 '17 at 11:23

3

I think a cleaner way to do so will be using a vector:

std::vector<unsigned  char> v; // vector with size 0
v.resize(r*c);                 // after size is known, just resize

answered Jun 01 '17 at 11:23

CIsForCookies

10,156
5
36
74

Is it bad practice to reinitialize a pointer?

2 Answers2