Copy Constructor and Destructor with a Dynamic Array

Question

I've been working on a project in which I am supposed to recreate the basics of the function using a dynamic array. The program should have a default constructor, which creates an array with a size of 2, and the "vector"'s size is 0 until more values are added to the vector class, named DynArray. When there's an attempt to .push_back into the vector when the array is at capacity, my program should make a new coppy of the array with 2x the capacity, and delete the old array. This is what I have so far, and I'm struggling to figure out how the copy constructor should be set up so it works appropriately.

I've been jumping between working on getting push_back and the copy constructor working properly when I've run into a wall with either in the mean time.

#include <iostream>
using namespace std;

class DynArray
{
   public:
      DynArray();
      DynArray(const DynArry& origClass);
      ~DynArray();
      int capacity();
      int size();
      void push_back(int newNum);
      int at(int atNum);

   private:
      int arraySize;
      int arrayCapacity;
      int newNum;
      int pushCounter;
      int atNum;
      int i;
      int arrayVector [];
};


DynArray::DynArray() // default constructor, creates array with no elements and capacity 2
{
   int* arrayVector = new int[2];
   arrayCapacity = 2;
   arraySize = 0;
   pushCounter = 0; // used for push_back, increments through array every time a new number is added with push_back
   return;
}

DynArray::DynArray(const DynArray& origClass) // copy constructor
{
   cout << "Copy Constructor Called." << endl;
   int* arrayVector = new int[2];
   *arrayVector = *(origClass.arrayVector)

   return;
}

void DynArray::push_back(int newNum)
{
   if (arraySize == arrayCapacity) // if the capacity is the same as the current size, make a new array with twice the capacity, copy over values, delete the old array
   {
      arrayCapacity = arrayCapacity * 2;
      int* newarrayVector = new int[arrayCapacity];
      for (i = 0; i < arraySize; ++i)
      {
         *newarrayVector [i] = arrayVector[i];
      }      
   arrayVector [pushCounter] = newNum; // push value to the next open value in the array
   ++arraySize; // increment so next push_back uses the following value
   ++pushCounter;
}

set the size in the copy constructor to the size needed. new int[orgClass.arrayCapacity] — Geoduck
– Geoduck, Commented Mar 31, 2018 at 1:01
The copy constructor needs to copy the whole array, just like your push_back function does when it allocates new memory. Also, you can remove newNum from your class declaration, it's a parameter passed to your function and does not need to be declared there. Remove pushCounter as well and use arraySize. — super
– super, Commented Mar 31, 2018 at 1:03
Future issue: No assignment operator. This breaks the Rule of Three and can really hit where it hurts. — user4581301
– user4581301, Commented Mar 31, 2018 at 1:24
The way this question is worded currently basically implies that you're asking someone to code it for you. Be more specific on why you're struggling. Also, there are a lot of problems in your code. pushCounter is useless since it does the exact same thing as arraySize. You have a member called arrayVector that stores the pointer to the dynamic array, but you then declare local variables with the same name in your constructor and push_back functions. You should also learn about constructor member initialization lists instead of assigning stuff to member variables. — user9254539
– user9254539, Commented Mar 31, 2018 at 1:57
Why can't you just allocate a new array and copy every element over? newNum and atNum are member function arguments, not class members. — user9254539
– user9254539, Commented Mar 31, 2018 at 1:59

Peter · Accepted Answer · 2018-03-31 05:46:42Z

Your copy constructor should make a copy the object passed to it.

Instead it does this;

DynArray::DynArray(const DynArray& origClass) // copy constructor
{
    cout << "Copy Constructor Called." << endl;
    int* arrayVector = new int[2];
    *arrayVector = *(origClass.arrayVector);   // missing ; added here

    return;
}

Ignoring output to cout, the two obvious problems are that arrayVector is dynamically allocated with two elements, regardless of how many elements there are in origClass.arrayVector.

The statement int *arrayVector = *(origClass.arrayVector) also does not copy an array at all.

First of all it creates a pointer named arrayVector (which is unrelated to the member of DynArray, despite having the same name).
Then it copies the first element of origClass.arrayVector (a single int) to the first element of arrayVector. So it is functionally equivalent to arrayVector[0] = origClass.arrayVector[0]. That's the way pointers work.
Lastly, when the constructor returns, the arrayVector declared ceases to exist, because if is local to the body of the constructor. A consequence is that the dynamically allocated memory is leaked (no variables, or members of your DynArray class, refer to it at all, so there is no way in standard C++ to find that memory in subsequent code).

Lastly, your copy constructor does not copy any other members of origClass (arraySize, arrayCapacity, newNum, pushCounter, atNum, i) to the new object being constructed.

Instead, what you (typically) need to do is copy ALL of of the members, not just one of them. And, if one of the members being copied is a dynamically allocated array - as in your case - the constructor needs to first construct a new array, and THEN copy all elements to it - one at a time, in a loop of some form.

For example, you could do

DynArray::DynArray(const DynArray& origClass) // copy constructor
{
    arrayVector = new int[origClass.arrayCapacity];

This actually modifies the arrayVector member of the DynaArray (rather than declaring a local variable). It also makes that pointer point at a dynamically allocated array of the same size as in origClass.

What this hasn't done is copy data from origClass.arrayVector. AS I noted above, this cannot be done using *arrayVector = *(origClass.arrayVector). Instead the constructor needs to do this

   for (int i = 0; i < origClass.arrayCapacity; ++i)
       arrayVector[i] = origClass.arrayVector[i];

This step is essential to ensure ALL the elements of origClass.arrayVector are copied. There are variations of HOW this is done (e.g. using pointer syntax rather than array syntax) but the above will suffice.

Then it is necessary to copy all of the other members from origClass.

   arraySize = origClass.arraySize;
   arrayCapacity = origClass.arrayCapacity;
   newNum = origClass.newNum;
   pushCounter = origClass.pushCounter;
   atNum = origClass.atNum;
   i = origClass.i;

}   // the body of the constructor ends here.

As an aside, it strikes me you have given your DynArray a number of members it doesn't need. Some of those members are only needed as temporaries in particular member functions (e.g. loop counters). Doing that means EVERY instance of DynArray has its own copy of those variables. That is unnecessary - they would be better off being local variables, declared in each function where they are needed.

Now, the above will (sort of) work, but needs to be improved. It is generally better to use initialisers in constructors where possible, and have the constructor body only do things that can't be done in initialisers. There are various advantages to doing this, but you can do homework to find out what they are.

In the end, therefore, I would implement the copy constructor as

DynArray::DynArray(const DynArray& origClass) :  // initialiser list begins here
        arrayVector(new int[origClass.arrayCapacity]),
        arraySize(origClass.arraySize),
        arrayCapacity(origClass.arrayCapacity),
        newNum(origClass.newNum), 
        pushCounter(origClass.pushCounter),
        atNum(origClass.atNum),
        i(origClass.i)
{
     // body of constructor starts here

     for (int i = 0; i < origClass.arrayCapacity; ++i)
         arrayVector[i] = origClass.arrayVector[i];
}

Not withstanding my previous aside that you have unnecessary members of the class, the initialiser list here still copies ALL of your DynArray members.

Note that the loop to copy array elements is in the body of the constructor, since that sort of thing cannot be done readily in an initialiser list.

Also note that the variable i (the loop counter in the constructor body) has no relationship to member of DynArray named i.

Lastly, the member of DynArray

int arrayVector [];

would be better declared as a pointer. It is not actually an array.

int *arrayVector;

Geoduck · Accepted Answer · 2018-03-31 01:09:01Z

0

Your copy constructor can access private members of the object it is copying, so it should be very much like your resize method. There are shortcuts (memcpy, std::copy) but you are better off learning how to use the loop.

So, list of little issues you have:

in copy constructor, set the size to the same size as the original object. (although, you could potentially shrink it to arraySize).
copy all the elements using a loop or bulk copy function (memcpy)
You are missing a } in your push_back method
pushCounter is always exactly the same as arraySize. You don't need both
i should be a local variable, not a class member.
The class members atNum and newNum are not used.

answered Mar 31, 2018 at 1:09

Geoduck

9,0212 gold badges29 silver badges27 bronze badges

Comments

Community · Accepted Answer · 2020-06-20 09:12:55Z

Few obvious bugs:

DynArray(const DynArry& origClass);
                     ^

This is probably a typo. There is no such type as DynArry.

 int arrayVector [];

This member declaration is ill formed. You are not allowed to have member arrays of unspecified size. Since you're re-implementing a dynamic array, this should probably instead be a pointer to hold the address of the dynamically allocated array. A pointer is declared like this:

  int* arrayVector;

int* arrayVector = new int[2];

This stores the address of the dynamic array in a local automatic variable. As soon as the constructor ends, the pointer - being automatic - is destroyed and the array is leaked. You should probably make the changes to this->arrayVector instead.

*arrayVector = *(origClass.arrayVector)

This copies only the first element of the array. The copy constructor should probably copy all of the elements. To that end, you should also construct a dynamic array that is as big as the one you're copying from.

Also, the copy constructor leaves many of the members uninitialized (such as arraySize).

 *newarrayVector [i] = arrayVector[i];

This expression is ill-formed. You cannot use the indirection operation on an integer. You probably intended to assign to the array element instead:

newarrayVector [i] = arrayVector[i];

Besides that, your reallocation suffers from the same problem as your constructors. The new array is simply leaked when the function ends since you don't store it in a member.

*arrayVector = *(origClass.arrayVector)

return;

This expression is ill-formed. You may have forgotten a semicolon.

Collectives™ on Stack Overflow

Copy Constructor and Destructor with a Dynamic Array

3 Answers 3

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related