[c++] Dynamically allocating an array of objects

I have a class that contains a dynamically allocated array, say

class A
{
    int* myArray;
    A()
    {
        myArray = 0;
    }
    A(int size)
    {
        myArray = new int[size];
    }
    ~A()
    {
        // Note that as per MikeB's helpful style critique, no need to check against 0.
        delete [] myArray;
    }
}

But now I want to create a dynamically allocated array of these classes. Here's my current code:

A* arrayOfAs = new A[5];
for (int i = 0; i < 5; ++i)
{
    arrayOfAs[i] = A(3);
}

But this blows up terribly. Because the new A object created (with the A(3) call) gets destructed when the for loop iteration finishes, and this means that the internal myArray of that A instance gets delete []-ed.

So I think my syntax must be terribly wrong? I guess there are a few fixes that seem like overkill, which I'm hoping to avoid:

• Creating a copy constructor for A.
• Using vector<int> and vector<A> so I don't have to worry about all this.
• Instead of having arrayOfAs be an array of A objects, have it be an array of A* pointers.

I would think this is just some beginners thing where there's a syntax that actually works when attempting to dynamically allocate an array of things that have internal dynamic allocation.

(Also, style critiques appreciated, since it's been a while since I did C++.)

Update for future viewers: All of the answers below are really helpful. Martin's is accepted because of the example code and the useful "rule of 4," but I really suggest reading them all. Some are good, succinct statements of what's wrong, and some point out correctly how and why vectors are a good way to go.

You need an assignment operator so that:

arrayOfAs[i] = A(3);

works as it should.

Why not have a setSize method.

A* arrayOfAs = new A[5];
for (int i = 0; i < 5; ++i)
{
    arrayOfAs[i].SetSize(3);
}

I like the "copy" but in this case the default constructor isn't really doing anything. The SetSize could copy the data out of the original m_array (if it exists).. You'd have to store the size of the array within the class to do that.
OR
The SetSize could delete the original m_array.

void SetSize(unsigned int p_newSize)
{
    //I don't care if it's null because delete is smart enough to deal with that.
    delete myArray;
    myArray = new int[p_newSize];
    ASSERT(myArray);
}

You need an assignment operator so that:

arrayOfAs[i] = A(3);

works as it should.

1. Use array or common container for objects only if they have default and copy constructors.

2. Store pointers otherwise (or smart pointers, but may meet some issues in this case).

PS: Always define own default and copy constructors otherwise auto-generated will be used

Using the placement feature of new operator, you can create the object in place and avoid copying:

placement (3) :void* operator new (std::size_t size, void* ptr) noexcept;

Simply returns ptr (no storage is allocated). Notice though that, if the function is called by a new-expression, the proper initialization will be performed (for class objects, this includes calling its default constructor).

I suggest the following:

A* arrayOfAs = new A[5]; //Allocate a block of memory for 5 objects
for (int i = 0; i < 5; ++i)
{
    //Do not allocate memory,
    //initialize an object in memory address provided by the pointer
    new (&arrayOfAs[i]) A(3);
}

I'd recommend using std::vector: something like

typedef std::vector<int> A;
typedef std::vector<A> AS;

There's nothing wrong with the slight overkill of STL, and you'll be able to spend more time implementing the specific features of your app instead of reinventing the bicycle.

Why not have a setSize method.

A* arrayOfAs = new A[5];
for (int i = 0; i < 5; ++i)
{
    arrayOfAs[i].SetSize(3);
}

I like the "copy" but in this case the default constructor isn't really doing anything. The SetSize could copy the data out of the original m_array (if it exists).. You'd have to store the size of the array within the class to do that.
OR
The SetSize could delete the original m_array.

void SetSize(unsigned int p_newSize)
{
    //I don't care if it's null because delete is smart enough to deal with that.
    delete myArray;
    myArray = new int[p_newSize];
    ASSERT(myArray);
}

You need an assignment operator so that:

arrayOfAs[i] = A(3);

works as it should.

I'd recommend using std::vector: something like

typedef std::vector<int> A;
typedef std::vector<A> AS;

There's nothing wrong with the slight overkill of STL, and you'll be able to spend more time implementing the specific features of your app instead of reinventing the bicycle.

1. Use array or common container for objects only if they have default and copy constructors.

2. Store pointers otherwise (or smart pointers, but may meet some issues in this case).

PS: Always define own default and copy constructors otherwise auto-generated will be used

The constructor of your A object allocates another object dynamically and stores a pointer to that dynamically allocated object in a raw pointer.

For that scenario, you must define your own copy constructor , assignment operator and destructor. The compiler generated ones will not work correctly. (This is a corollary to the "Law of the Big Three": A class with any of destructor, assignment operator, copy constructor generally needs all 3).

You have defined your own destructor (and you mentioned creating a copy constructor), but you need to define both of the other 2 of the big three.

An alternative is to store the pointer to your dynamically allocated int[] in some other object that will take care of these things for you. Something like a vector<int> (as you mentioned) or a boost::shared_array<>.

To boil this down - to take advantage of RAII to the full extent, you should avoid dealing with raw pointers to the extent possible.

And since you asked for other style critiques, a minor one is that when you are deleting raw pointers you do not need to check for 0 before calling delete - delete handles that case by doing nothing so you don't have to clutter you code with the checks.

You need an assignment operator so that:

arrayOfAs[i] = A(3);

works as it should.

I'd recommend using std::vector: something like

typedef std::vector<int> A;
typedef std::vector<A> AS;

There's nothing wrong with the slight overkill of STL, and you'll be able to spend more time implementing the specific features of your app instead of reinventing the bicycle.

Using the placement feature of new operator, you can create the object in place and avoid copying:

placement (3) :void* operator new (std::size_t size, void* ptr) noexcept;

Simply returns ptr (no storage is allocated). Notice though that, if the function is called by a new-expression, the proper initialization will be performed (for class objects, this includes calling its default constructor).

I suggest the following:

A* arrayOfAs = new A[5]; //Allocate a block of memory for 5 objects
for (int i = 0; i < 5; ++i)
{
    //Do not allocate memory,
    //initialize an object in memory address provided by the pointer
    new (&arrayOfAs[i]) A(3);
}

Why not have a setSize method.

A* arrayOfAs = new A[5];
for (int i = 0; i < 5; ++i)
{
    arrayOfAs[i].SetSize(3);
}

I like the "copy" but in this case the default constructor isn't really doing anything. The SetSize could copy the data out of the original m_array (if it exists).. You'd have to store the size of the array within the class to do that.
OR
The SetSize could delete the original m_array.

void SetSize(unsigned int p_newSize)
{
    //I don't care if it's null because delete is smart enough to deal with that.
    delete myArray;
    myArray = new int[p_newSize];
    ASSERT(myArray);
}

The constructor of your A object allocates another object dynamically and stores a pointer to that dynamically allocated object in a raw pointer.

For that scenario, you must define your own copy constructor , assignment operator and destructor. The compiler generated ones will not work correctly. (This is a corollary to the "Law of the Big Three": A class with any of destructor, assignment operator, copy constructor generally needs all 3).

You have defined your own destructor (and you mentioned creating a copy constructor), but you need to define both of the other 2 of the big three.

An alternative is to store the pointer to your dynamically allocated int[] in some other object that will take care of these things for you. Something like a vector<int> (as you mentioned) or a boost::shared_array<>.

To boil this down - to take advantage of RAII to the full extent, you should avoid dealing with raw pointers to the extent possible.

And since you asked for other style critiques, a minor one is that when you are deleting raw pointers you do not need to check for 0 before calling delete - delete handles that case by doing nothing so you don't have to clutter you code with the checks.

The constructor of your A object allocates another object dynamically and stores a pointer to that dynamically allocated object in a raw pointer.

For that scenario, you must define your own copy constructor , assignment operator and destructor. The compiler generated ones will not work correctly. (This is a corollary to the "Law of the Big Three": A class with any of destructor, assignment operator, copy constructor generally needs all 3).

You have defined your own destructor (and you mentioned creating a copy constructor), but you need to define both of the other 2 of the big three.

An alternative is to store the pointer to your dynamically allocated int[] in some other object that will take care of these things for you. Something like a vector<int> (as you mentioned) or a boost::shared_array<>.

To boil this down - to take advantage of RAII to the full extent, you should avoid dealing with raw pointers to the extent possible.

And since you asked for other style critiques, a minor one is that when you are deleting raw pointers you do not need to check for 0 before calling delete - delete handles that case by doing nothing so you don't have to clutter you code with the checks.

I'd recommend using std::vector: something like

typedef std::vector<int> A;
typedef std::vector<A> AS;

There's nothing wrong with the slight overkill of STL, and you'll be able to spend more time implementing the specific features of your app instead of reinventing the bicycle.

The constructor of your A object allocates another object dynamically and stores a pointer to that dynamically allocated object in a raw pointer.

For that scenario, you must define your own copy constructor , assignment operator and destructor. The compiler generated ones will not work correctly. (This is a corollary to the "Law of the Big Three": A class with any of destructor, assignment operator, copy constructor generally needs all 3).

You have defined your own destructor (and you mentioned creating a copy constructor), but you need to define both of the other 2 of the big three.

An alternative is to store the pointer to your dynamically allocated int[] in some other object that will take care of these things for you. Something like a vector<int> (as you mentioned) or a boost::shared_array<>.

To boil this down - to take advantage of RAII to the full extent, you should avoid dealing with raw pointers to the extent possible.

And since you asked for other style critiques, a minor one is that when you are deleting raw pointers you do not need to check for 0 before calling delete - delete handles that case by doing nothing so you don't have to clutter you code with the checks.

1. Use array or common container for objects only if they have default and copy constructors.

2. Store pointers otherwise (or smart pointers, but may meet some issues in this case).

PS: Always define own default and copy constructors otherwise auto-generated will be used