How do you copy an object of a class to another object of the same class by using '='

Question

How do you copy an object of a class to another object of the same class just by using '='. I know that we have to overload the operator. here's what I have so far

#include<iostream>
#include<conio.h>
#include<iomanip>

using namespace std;



class arr
{
public:
    int *arr1;
    int len;

    arr& operator = (const arr& eq) //for copying two arrays. <--- my overloader
    {
        arr temp1(eq.len);
        arr *pttemp;
        int i=0;
        //temp.arr1=new int[eq.len];
        //temp.len = eq.len;
        for(i = 0 ; i < eq.len ; i++)
        {
            temp1.arr1[i] = eq.arr1[i];
        }       
        pttemp = &temp1;
        return temp1;
    };

    friend istream& operator >> (istream& ist, arr & r)
    {
        static int i = 0;
        int *arrNew;

        if (i == r.len)
        {
            r.len *=2;
            arrNew = new int[r.len]; // allocate the new array              
            for(int j = 0; j < r.len/2; j++)// copy the old array to the first half of the new array
            arrNew[j] = r.arr1[j];// delete the old array         
            delete [] r.arr1;// let arr point to the new array and continue use arr     
            r.arr1 = arrNew;
            delete arrNew;
        }

        ist>>r.arr1[i];
        i++;

        return ist;
    }

    arr() //initializing constructor
    {
        len = 5;
        arr1 = new int[len];
    };

    arr(int size) //initializing constructor with args
    {
        len = size;
        arr1 = new int[len];
    };

    arr(arr& a) : arr1(a.arr1) //copy constructor
    {
        arr1 = new int[len];
    };

    ~arr() //delete constructor
    {
        delete arr1;
    };



};




void main()
{
    int size = 5,i,temp,trig = 0;
    arr orig(size), asc(size), desc(size);  

    //generate random numbers for orig

    for (i = 0 ; i < size ; i++)
    {
        orig.arr1[i] = rand();
    }

    //copy original set to asc and desc
    asc = orig;
    desc = orig;

    //sorting ascending
    for (i = 0 ; i < size-1 ; i++)
    {
        trig = 1;
        if (asc.arr1[i] < asc.arr1[i+1])
        {
            temp = asc.arr1[i];
            asc.arr1[i] = asc.arr1[i+1];
            asc.arr1[i+1] = temp;
            trig = 0;
        }
        if (trig = 1)
            break;
        if (i == size - 1)
        {
            i = 0;
        }
    }

    //sorting descending
    for (i = 0 ; i < size-1 ; i++)
    {
        trig = 1;
        if (desc.arr1[i] > desc.arr1[i+1])
        {
            temp = desc.arr1[i];
            desc.arr1[i] = desc.arr1[i+1];
            desc.arr1[i+1] = temp;
            trig = 0;
        }
        if (trig = 1)
            break;
        if (i == size - 1)
        {
            i = 0;
        }
    }

    //printing  
    cout<<"Original Array: ";
    for (i = 0 ; i < size ; i++)
    {
        cout<<orig.arr1[i]<<" ";
    }
    cout<<endl;
    cout<<"Ascending Array: ";
    for (i = 0 ; i < size ; i++)
    {
        cout<<asc.arr1[i]<<" ";
    }
    cout<<endl;

    cout<<"Descending Array: ";
    for (i = 0 ; i < size ; i++)
    {
        cout<<desc.arr1[i]<<" ";
    }
    cout<<endl;

    getch();

}

It compiles properly but it ('asc' and 'desc') displays numbers that are different from the 'orig' object.

Use [copy and swap](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) method. — Alok Save, Mar 02 '12 at 05:38
Somewhat unrelated, and it might just be because it's not complete yet, but the copy constructor isn't copying anything. — R. Martinho Fernandes, Mar 02 '12 at 05:43

score 2 · Answer 1 · answered Mar 02 '12 at 06:18

The proper solution is something like this:

struct Foo
{
    std::vector<int> arr;
    friend std::ifstream & operator>>(/*...*/);
};

The implicitly defined assignment operator already does exactly what you need, and clever code reuse is the heart of C++ programming ("a language for library design").

If you want to write it by hand, you have to make the copy yourself:

struct Bar
{
    unsigned int len;
    int * arr;

    Bar & operator=(Bar const & rhs)
    {
        len = rhs.len; 
        delete[] arr;
        arr = new int[len];
        for (unsigned int i = 0; i != len; ++i) { arr[i] = rhs.arr[i]; }
        return *this;
    }

    Bar() : len(), arr() { }
    // more constructors

    Bar(Bar const &);  // exercise: write this!

    ~Bar() { delete[] arr; }
};

This is a terrible, terrible idea, though, since this pattern doesn't generalize at all: it isn't exception safe -- imagine one of the copies in the for-loop threw an exception. Now you've lost your original data and leaked memory.

A better solution would be to allocate a temporary buffer first:

int * temp = new int[len];
for (...) temp[i] = rhs.arr[i];

delete[] arr;
arr = temp;

Now this code is quickly getting very ugly, and imagine you had more than one of those!

In a nutshell: use std::vector.

"A better solution would be to allocate a temporary buffer first" - if you're going to do that then you might as well copy-and-swap. In this particular case the assignments in the `for` loop can't throw, so there is a potential optimization there if requirements demand it, to avoid a memory allocation in the case where the existing buffer is big enough. For exception safety you'd also need to copy `len` *after* doing the memory allocation. I still agree, though, use `vector` until you've proven some problem with it. — Steve Jessop, Mar 02 '12 at 08:59

score 0 · Answer 2 · answered Mar 02 '12 at 06:17

You don't need a temporary array object in the assignment operator, and you should copy to the array in this, and then return *this:

arr &operator=(const arr &eq)
{
    // If "this" already has an array, then delete it
    if (arr1)
        delete [] arr1;

    // Create a new array of the same length as the one we're assigning from
    len = eq.len;
    arr1 = new int [len];

    // And copy the array
    memcpy(arr1, eq.arr1, len * sizeof(int));

    // Returning "*this" allows the assignment operator to be chained
    return *this;
}

this solution is not exception safe. What if allocation of memory to arr1 threw an exception ? — Jagannath, Mar 02 '12 at 06:35

score 0 · Answer 3 · answered Mar 02 '12 at 06:21

what you have implemented in operator overloading is quite confusing and seems wrong to me

arr& operator = (const arr& eq) //for copying two arrays. <--- my overloader
    {
        arr temp1(eq.len);
        arr *pttemp;
        int i=0;
        //temp.arr1=new int[eq.len];
        //temp.len = eq.len;
        for(i = 0 ; i < eq.len ; i++)
        {
            temp1.arr1[i] = eq.arr1[i];
        }       
        pttemp = &temp1;
        return temp1;
    };

1.why are you creating a new object temp?

2.why create a pointer of type arr. and assigning pttemp to point to temp whose scope is with in the function and then you are not using it anywhere!!

you need not create a new object inside the function as "this" pointer is implicitly passed to the function.

you should overload it this way

arr& operator = (const arr& source)
{
 //this->len = source.len;
 len =  source.len;

for(int i=0;i < source.len ; i++)
{
//this->arr1[i] = source.arr1[i];
arr1[i] = source.arr1[i];
}

return *this;
}

@Jagannath how can it be he has initialized it in the main() when he declared the objects; — ken, Mar 02 '12 at 06:34
arr obj1(5); arr obj2(10); obj1 = obj2; What happens here with your code ? — Jagannath, Mar 02 '12 at 08:40

Steve Jessop · Answer 4 · 2012-03-02T11:21:16.717

Your copy constructor also looks wrong, it doesn't copy the contents of the array.

I agree with Kerrek that you should use vector. But since you appear to be trying to re-implement vector for yourself, here's a simple "correct" way to manage the resources in your class, for illustration:

class arr
{
  public:
    // switch order of data members, we want to initialize "len" first
    int len;
    int *arr1;

    // default constructor
    // you could use "new int[len]()" instead, to zero-initialize the array
    arr() : len(5), arr1(new int[len]) {}

    // constructor with size
    arr(int size) : len(size), arr1(new int[len]) {}

    // copy constructor
    arr(const arr &rhs) : len(rhs.len), arr1(new int[len]) {
        std::copy(rhs.arr1, rhs.arr1 + len, arr1);
    }

    // destructor
    ~arr() {
        delete[] arr1; // *not* "delete", you *must* use "delete[]"
    }

    // swap function (this is useful, observe that it cannot throw)
    void swap(arr &rhs) {
        std::swap(len, rhs.len);
        std::swap(arr1, rhs.arr1);
    }

    // assignment operator
    arr &operator=(arr temp) { // parameter by value uses the copy ctor
        // so, anything that could throw (the allocation) is complete,
        // before we make any modifications to this object.
        // see how useful "swap" is, and the fact it cannot throw?
        swap(temp);
        return *this;
    }

    // for C++11
    // move constructor
    arr(arr &&rhs) : len(rhs.len), arr1(rhs.arr1) {
        rhs.arr1 = 0;
    }
};

Btw, the name of the data member arr1 makes me fear that you're going to add a second array later. Do not do this, it's far more trouble to write a class that correctly manages two resources, than it is to write a class that correctly manages one resource and another class that has two of those as data members.

How do you copy an object of a class to another object of the same class by using '='

4 Answers4