Dynamic resizeable array

How can I improve this code for a dynamic and re-sizable array data structure?

Array.h

#ifndef _ARRAY_H_
#define _ARRAY_H_

class Array
{
private:
    int * m_ArrayContainer;//These two variables
    int m_Size;            //work in pairs.

private:
    void SetSize(int size);     
    void AllocateMemoryOfSize(int size);
    void DeallocateMemory();        

public:
    Array();
    Array(int size);
    Array(Array & arr);
    void operator = (Array & arr);      
    void Resize(int newSize);
    int GetSize();
    void SetItem(int index, int value);
    int GetItem(int index);
    ~Array();
};

#endif

Array.cpp

```
#include "Array.h"
#include

#pragma region Private Methods

void Array :: SetSize(int size)
{
this->m_Size = size;
}
void Array :: AllocateMemoryOfSize(int size)
{
this->m_ArrayContainer = new int[size];
this->SetSize(size);
}
void Array :: DeallocateMemory()
{
if(this->GetSize() > 0)
{
delete [] this->m_ArrayContainer;
this->SetSize(0);
}
}
void Array :: SetItem(int index, int value)
{
this->m_ArrayContainer[index] = value;
}
#pragma endregion

Array :: Array()
{
//std::coutm_ArrayContainer = NULL;

this->SetSize(0);
}
Array :: Array(int size)
{
//std::coutSetSize(0);

this->Resize(size);
}
void Array :: Resize(int newSize)
{
int oldSize = this->GetSize();

for(int i=0 ; iSetItem(i, NULL);
}
this->DeallocateMemory();

this->AllocateMemoryOfSize(newSize);
this->SetSize(newSize);
}
Array :: Array(Array & arr)
{
//std::coutSetSize(0);

int size = arr.GetSize();

this->Resize(size);

for(int i=0 ; iSetItem(i, arr.GetItem(i));
}
}
void Array :: operator = (Array & arr)
{
//std::coutSetSize(0);

int size = arr.GetSize();

this->Resize(size);

for(int i=0 ; iSetItem(i, arr.GetItem(i));
}
}
int Array :: GetSize()
{
return this->m_Size;
}
int Array :: GetItem(int index)
{
r

You have a couple of memory leaks.

First, the DeallocateMemory() will only delete the object's memory block if the size is greater than 0:

void Array :: DeallocateMemory()
{           
    if(this->GetSize() > 0)  // GetSize() will return zero at this point
    {
        delete [] this->m_ArrayContainer;
        this->SetSize(0);       
    }
}

However, in C++ it's legal to allocate a memory block with 0 size. (See the Q&A C++ new int[0] -- will it allocate memory? on Stack Overflow for more information.) When you allocate memory, you actually get two things: the memory that you asked for, and some data that the C++ runtime uses to keep track of that memory block.

If you have a sequence of code like:

Array a(0);
a.Resize(1);

the constructor will allocate a 0-sized block of memory and the extra data for the C++ runtime. When Resize() calls DeallocateMemory(), it'll see that your data block is 0-sized, won't invoke delete[], and leak that memory when AllocateMemoryOfSize() overwrites the object's m_ArrayContainer pointer:

void Array :: AllocateMemoryOfSize(int size)
{
    this->m_ArrayContainer = new int[size]; // Memory leak!
    this->SetSize(size);
}

Second, in your assignment operator, the first thing you do is to set the size of this to zero:

void Array :: operator = (Array & arr)
{
    this->SetSize(0);
    int size = arr.GetSize();       
    this->Resize(size);
    /// [stuff elided].
}

Next, you call Resize with the size of the other object:

void Array :: Resize(int newSize)
{
    int oldSize = this->GetSize();

    for(int i=0 ; iSetItem(i, NULL);
    }
    this->DeallocateMemory();

    this->AllocateMemoryOfSize(newSize);
    this->SetSize(newSize);
}

Note that the for-loop at the start doesn't actually do anything because the size of this has been set to zero by the time you get here. Even if it didn't, it would be unnecessary because the intent is to delete[] that memory soon afterwards, and once it has been returned to the heap, the contents of memory are irrelevant.

Then you call DeallocateMemory(), which won't delete any memory because the size field in this has already been set to zero. Finally, you call AllocateMemoryOfSize() which unconditionally overwrites the pointer to the array, leaking memory the same way as in my first point above.

To fix these bugs, I would:

-
Check if m_ArrayContainer is a null pointer to decide if I need to delete[] it.

void Array :: DeallocateMemory()
{           
    if(this->m_ArrayContainer != NULL)
    {
        delete [] this->m_ArrayContainer;
        this->m_ArrayContainer = NULL;
        this->SetSize(0);       
    }
}

-
Don't zero out the memory in this just before deleting it; your Resize() method then becomes:

void Array :: Resize(int newSize)
{
    this->DeallocateMemory();

    this->AllocateMemoryOfSize(newSize);
    this->SetSize(newSize);
}

-
Don't set the size field independently of the allocation or deallocation of memory. You have calls to SetSize() in several places in your code: remove all except the ones in AllocateMemoryOfSize() and DeallocateMemory().

In the context of your assignment operator, this becomes:

void Array :: operator = (Array & arr)
{
    int size = arr.GetSize();       

    this->Resize(size);

    for(int i=0 ; iSetItem(i, arr.GetItem(i));
    }
}

Note that there are still other ways that this class could fail, for example if an exception is thrown during allocation. I recommend reading about exception guarantees: in a nutshell, do the memory allocation and copying of the other instance into temporary memory first, then if that succeeds, free up the resources in this instance and assign the temporary values to them. Scott Meyers covers it well in his book, Effective C++.

Speaking of the assignment operator, it's customary to have it return a reference to this:

Array &Array::operator= (Array & arr)
{
    // [rest of the assignment operator elided]
    return *this;
}

This will allow you to chain together assignments like:

Array a, b, c;
// ... stuff ...
a = b = c;

StackExchange Code Review Q#94935, answer score: 4