Copy constructor in c++?

Question

So I have to write an operator= method in c++ that copies all the values of one array into another. Here's what I wrote:

dynamic_array &dynamic_array::operator=(const dynamic_array &a) {
    size = a.get_size();
    if (size % BLOCK_SIZE == 0){ //a multiple of BLOCK_SIZE
        allocated_size = size;
    } else {
        int full_blocks = size / BLOCK_SIZE;
        allocated_size  = (full_blocks+1) * BLOCK_SIZE;
    }
    try {
        array = new int[allocated_size];
    } catch (bad_alloc){
        throw exception (MEMORY_EXCEPTION);
    }

    //copy a[i..size-1]
    for (int i = 0; i < size; i++){
        array[i] = a[i];
    }
    return *this; //returns a reference to the object
}

So it doesn't assume anything about the sizes and sets the size and allocated size of the array it's given (and using another get_size() method). Now the second code I have to write just says I have to create an array containing a copy of the elements in a. Now I just wrote the same thing as I did for my operator= method (without returning anything):

dynamic_array::dynamic_array(dynamic_array &a) {
    size = a.get_size();
    if (size % BLOCK_SIZE == 0){ //a multiple of BLOCK_SIZE
        allocated_size = size;
    } else {
        int full_blocks = size / BLOCK_SIZE;
        allocated_size  = (full_blocks+1) * BLOCK_SIZE;
    }
    try {
        array = new int[allocated_size];
    } catch (bad_alloc){
        throw exception (MEMORY_EXCEPTION);
    }

    //copy a[i..size-1]
    for (int i = 0; i < size; i++){
        array[i] = a[i];
    }
}

Now this is giving me the output that I want but I'm just wondering if there's an easier way to do these two methods. The methods are for a dynamic array and I feel like there's more lines of code than needed. The operator= one is supposed to copy the elements of a into a new dynamic array and dynamic_array::dynamic_array(dynamic_array &a) { is supposed to create a new array containing a copy of the elements in a. It sounds like the same code for each method because you always need to create a new array and you will always need to copy the array elements from one array to another but is there a more simpler way to write these two methods or is this the simplest way to do it?

Answer 1

You could simplify both the copy constructor and the copy assignment operator by changing the type of array to std::vector<int>.

The vector does all the work for you and you don't even need any custom implementations at all.

Answer 2

First to make this simpler, you can make use of the fact that you have access to all private members of a. Also the use of memcpy will make this perform better. A couple of lines are added as well to delete the existing array before copying.

dynamic_array &dynamic_array::operator=(const dynamic_array &a) {
    size = a.size;
    allocated_size = a.allocated_size;

    int* newArray = new int[allocated_size];

    //copy array
    memcpy(newArray, a.array, allocated_size * sizeof(int));

    // Delete existing array
    if (array != NULL)
        delete[] array;

    array = newArray;

    return *this; //returns a reference to the object
}

Secondly, for copy constructor, a trick we used to do is to call the equal operator inside the copy constructor in order not to repeat the code:

dynamic_array::dynamic_array(dynamic_array &a) : array(NULL) {
    *this = a;
}

Answer 3

From Scot Meyers (classic) book "Effective C++":

"In practice, the two copying functions will often have similar bodies, and this may tempt you to try to avoid code duplication by having one function call the other. Your desire to avoid code duplication is laudable, but having one copying function call the other is the wrong way to achieve it.

It makes no sense to have the copy assignment operator call the copy constructor, because you'd be trying to construct an object that already exists. This is so nonsensical, there's not even a syntax for it. There are syntaxes that look like you're doing it, but you're not; and there are syntaxes that do do it in a backwards kind of way, but they corrupt your object under some conditions. So I'm not going to show you any of those syntaxes. Simply accept that having the copy assignment operator call the copy constructor is something you don't want to do.

Trying things the other way around — having the copy constructor call the copy assignment operator — is equally nonsensical. A constructor initializes new objects, but an assignment operator applies only to objects that have already been initialized. Performing an assignment on an object under construction would mean doing something to a not-yet-initialized object that makes sense only for an initialized object. Nonsense! Don't try it.

Instead, if you find that your copy constructor and copy assignment operator have similar code bodies, eliminate the duplication by creating a third member function that both call. Such a function is typically private and is often named init. This strategy is a safe, proven way to eliminate code duplication in copy constructors and copy assignment operators."

And by the way, it'd be wise to also check for self-equality in you assignment operator.