1

In the below code I have a class A which has a dynamic array of ints. I have another class B which has an array of pointers to objects of class A.I have already written the copy constructor for class A. I need to write a copy constructor and destructor for class B, I have tried various approaches but with no success.

Definition of class A :

class A {
  public:
    A::A(const A& other){
        siz = other.siz;
        c = other.c;
        s = other.s;
        e = other.e;
        arr= new int[other.c];
        memcpy(arr, other.arr, sizeof(int) * c);
    }
    A::~A() {
        delete [] m_arr; 
    }

    const A& operator=(const A& rhs){

        if(this == &rhs)
            return *this; // handling of self assignment

        delete[] arr; // freeing previously used memory

        arr = new int[rhs.c];
        siz = rhs.siz;
        c = rhs.c;
        e = rhs.e;
        s = rhs.s;

        memcpy(m_arr, rhs.arr, sizeof(int) * c);
        return *this;
    }

  private :
    int *arr ;        
    int c ;
    int siz ;     
    int s ;     
    int e ;       
}

Definition of class B :

 class B {

      public:
        B::B(const B& other){
            // .......need help here
        }

        B::~B() {
           //......need help here
        }

      private :
        static const int constant = 7;
        A * array[constant] ;        
        int x ;
        int y ;     
        int z ;           
 }

Thanks for the help

  • 6
    why dont you use `std::vector` ? – 463035818_is_not_a_number Sep 26 '17 at 10:18
  • B::B(const A& other){, why class A here?It should be class B – Sumeet Sep 26 '17 at 10:20
  • Write an assignemnt op for A and I can help – doctorlove Sep 26 '17 at 10:31
  • @tobi303 I was trying to understand how pointers with arrays and objects work, hence i am not going for vectors. – swarnim singhal Sep 26 '17 at 10:31
  • @doctorlove I have already written one...i will edit the post and paste it here. – swarnim singhal Sep 26 '17 at 10:32
  • 1
    Do you want to share the `A`s pointed to in `B::array`, or do you want to copy them? P.S. you can't actually use either `A` or `B` as it stands, there is no way to initialise one. P.P.S. don't use names of types as names of variables. `A::arr` and `B::array` are entirely uninformative as to what they are for – Caleth Sep 26 '17 at 10:41
  • if you do not want to use `std::vector` then you should write something similar to `std::vector` once and not each time you need dynamic array, ie encapsulate all the memory allocation stuff in a class such that when using the class you dont need to worry about it (more or less what the answer is suggesting) – 463035818_is_not_a_number Sep 26 '17 at 11:25

2 Answers2

2

The key is to wrap raw owning pointers into RAII resource managers, and define classes assembling those safe RAII building blocks. Then, the C++ compiler will be able to automatically synthesize copy operations and destructors (and move operations as well).

For example, in your class A you have a int *arr data member, that you use to store an owning raw pointer to a dynamically allocated array. Replace it with a RAII resource manager, like the standard std::vector container (e.g. std::vector<int> arr).

Doing so, there's no need to define an explicit destructor, as the compiler will automatically invoke the std::vector destructor on your vector data member, and memory will be automatically released (without you invoking an explicit delete[]).

Similarly, the default copy constructor will do a member-wise copy, so the std::vector copy constructor will be automatically invoked by the C++ compiler, and the deep-copy from the source vector to the destination vector will automatically happen, without you "reinventing the wheel" with new[] dynamic allocations, memcpy, etc.

Starting with C++11, you can tell the C++ compiler to synthesize a default copy constructor with this syntax (available also for default constructor, move constructor, copy assignment operator, etc.)

class A {
public:
    ...
    A(const A&) = default;

    ...
};

The same goes for class B, instead of the A * array[constant] data member, consider using a vector, for example a vector of smart pointers to A:

vector<shared_ptr<A>> arrayOfAs;

An alternative might be using std::array for something of constant size.

Anyway, the key is: Consider using RAII resource managers as building blocks for more complex classes, instead of having raw owning pointers and other unsafe raw resources as data members. Each raw resource should be safely wrapped in its own RAII resource manager, and this in turn should be used as data member for more complex classes.


P.S. As a bonus reading consider reading "What is The Rule of Three? ", and this kind of follow up.

Mr.C64
  • 37,988
  • 11
  • 76
  • 141
  • I wonder how many of this (good in other cases) answer will the begineer understand.. :/ – gsamaras Sep 26 '17 at 10:37
  • @gsamaras I don't know. But I do know I did my best to write a clear and well elaborated answer (in my time constrains), and try to give some useful help to the OP. – Mr.C64 Sep 26 '17 at 10:46
2

Let's first assume that for this exercise, for whatever reason, you can't use std::vector or std::array. Assuming we follow the design of your classes so far, we can implement a copy constructor like this:

B::B(const B &other)
{
    for (std::size_t i = 0; i < constant; ++i) {
        // Use `new` to allocate memory and also call `A`'s copy constructor.
        array[i] = new A(*other.array[i]);
    }
}

What this does, as array is an array of pointers to As, is allocates dynamic memory for every element in the array, and populates the array with pointers to these dynamically allocated A objects using new, while also calling the copy constructor you have made for A, by using the syntax new A(other_a), which calls your A::A(const A &other). As other is a reference to an A, rather than a pointer to an A, the pointer held in other.array[i] is dereferenced, which is why the call is A(*other.array[i]), rather than A(other.array[i]).

As we have allocated dynamic memory here with new, the destructor must call delete for each of our calls to `new. This can be implemented similarly as so:

B::~B()
{
    for (std::size_t i = 0; i < constant; ++i) {
        // As each `A` has been allocated with `new`, they should now be
        // destroyed.
        delete array[i];
    }
}

So what we have now is something which seems to work as we want and we may assume that's all there is to it. However, things start to get complicated, as what would happen if one of the allocations performed by new fails and throws an exception? Or what if A's constructor throws an exception? delete will never be called on the elements which have been allocated with new so far.

To make our copy constructor exception safe requires some slightly more convoluted code. For example, something like this:

B::B(const B &other)
{
    std::size_t i;

    try {
        for (i = 0; i < constant; ++i) {
            array[i] = new A(*other.array[i]);
        }
    } catch (...) {
        // Delete all elements allocated so far
        for (std::size_t d = 0; d < i; ++d) {
            delete array[i];
        }

        // Re-throw the exception to the caller
        throw;
    }
}

Code like this can quickly become unmaintainable. To avoid such problems, a good guideline is that managing the lifetime of a resource which must be created and destroyed should be encapsulated by a class which only manages the lifetime of that one resource. This is important because if you start adding more constructs to your class similar to this array, then your class would be responsible for constructing and destructing even more than just this array, which would make exception safety even more difficult than it already is. In fact, the reason constructing and destructing your array is already quite convoluted is because your class is responsible for the lifetime of 7 separate resources (dynamically allocated objects), one for each array element.

With this in mind, a way to simplify this would be to use a class which encapsulates dynamically allocating and deallocating an object with new and delete. C++11 includes several classes which encapsulate the deallocation part at least, the most relevant being std::unique_ptr and std::shared_ptr. These classes however are designed to avoid copying. unique_ptr is explicitly non-copyable, and copying a shared_ptr just creates a new reference to the same resource, while keeping a reference count. This means you still have to manually implement copying.

You could switch to unique_ptr by changing your declaration in B from:

A *array[constant];

to:

std::unique_ptr<A> array[constant];

Then you could populate each member in your copy constructor with:

array[i] = std::unique_ptr<A>(new A(*other.array[i]));

With this approach, you would no longer have to worry about catching exceptions, as the destructor will be called automatically for each unique_ptr in the array if an exception is thrown somewhere in the constructor. The unique_ptrs which have not yet been assigned to will hold null pointers by default, and will safely do nothing when they are destroyed.

There is however one other approach: not using pointers / dynamic memory at all. You already have a class (A) which is responsible for its own resource's lifetime.

To do this, the following declaration in B can be changed from:

A *array[constant];

to:

A array[constant];

This would mean that you no longer need to define a copy constructor (or copy assignment operator) at all. If no copy constructor is provided in a C++ class, the class can be copied as though it has a simple memberwise copy constructor, which also works for arrays and will call the copy constructor for each element in the array. And as the array itself is part of the class and holds no pointers to dynamic memory, each element does not need to be manually deallocated with delete.

Candy Gumdrop
  • 2,651
  • 1
  • 16
  • 16