Compilation error due to copy constructor

Question

#include<iostream>

using namespace std;

class Abc
{
        public:

        int a;

        Abc()
        {
    cout<<"Def cstr called\n";
                a=0;
        }


        Abc(Abc &source)
        {
                a=source.a;
                cout<<"copy constructor is called"<<endl;
        }
        void operator = (Abc &source)
        {
                a=source.a;
                cout<<"overloaded assignment operator"<<endl;
        }
        Abc display(Abc ab)
        {
                cout<<"The a in display "<<ab.a<<endl;
                return ab;
        }
};

int main()

{

        Abc a;

        Abc b;

        a.a=10;

        b=a;

        cout<<"b.a ="<<b.a<<endl;

        Abc x;

        x = (a.display(b));

        return 0;

}

In the line x = (a.display(b)) I am getting the compilation error. On commenting this line it works. Please help in modifying the program so as to compile it successfully and please suggest me what wrong is going here.

http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three — Anonymous, Mar 30 '12 at 08:28

Michael Anderson · Accepted Answer · 2012-03-30T08:42:54.873

1

You should be passing your constructor and assignment arguments by const reference, rather than just by reference:

ie:

Abc(const Abc &source) { ... }
const Abc& operator=(const Abc &source) { ... }

You may (or may not) also want display to be like this:

const Abc& display(const Abc &ab)
{
        cout<<"The a in display "<<ab.a<<endl;
        return ab;
}

ASIDE: While these changes are not strictly required, they will prevent some tricky errors - one of which you've run into:

void operator = (Abc &source);
Abc display(Abc ab);

Abc a, b, x;

x = (a.display(b));

Here display is returning a temporary Abc, and operator= is taking a non-const Abc reference. One odd rule of C++ is that you can not bind a non-const reference to a temporary - hence your error.

edited Mar 30 '12 at 08:42

answered Mar 30 '12 at 08:33

Michael Anderson

70,661
7
134
187

Thanks Anderson...Now its working. Can you please explain something about it that why should we make it const. And why is the problem coming when its not const. Thanks !!! – Kundan Kumar Mar 30 '12 at 08:36
Thanks Anderson.....It means that if a function is returning a temporary value and if I need to pass that returned value to some other function then it should have parameters as constant reference. Is it just a convention ? Also, if I am making void operator = (Abc source) and Abc( const Abc &source) then it works but if in copy constructor I am keeping Abc( Abc &source) then it doesn't work. Sorry if I am bothering you..can you please clarify this. – Kundan Kumar Mar 30 '12 at 08:56

hmjd · Answer 2 · 2012-03-30T08:40:28.787

The compiler is raising an error because it is illegal to assign a temporary object to a non-const reference. The temporary in this case is the Abc object being returned by a.display() which is then passed to the assignment operator.

Make the arguments to the copy constructor and assignment operator const:

Abc(const Abc& source) { .. }
void operator=(const Abc& source) { ... }

Note, the assignment operator typically protects against self-assignment and returns a reference to itself:

Abc& operator=(const Abc& source)
{
    if (this != &source)
    {
        a = a.source; 
    }
    return *this;
}

score 1 · Answer 3 · answered Mar 30 '12 at 08:35

Your operator= doesn't return anything. It may not seem intuitive at first, but you need to return a reference to *this, which is what lets you do things like a = b = c.

const Abc & operator = (const Abc &source)
{
    a=source.a;
    cout<<"overloaded assignment operator"<<endl;
    return *this;
}

Also, convention is to use const references (since you're not expected to change the original object in the copy constructor or operator=).

Abc(const Abc &source) { ... }
...
Abc & display(Abc &ab) { ... }

I changed the return type and parameters of display too. Basically, use references whenever you can. Since you're returning the same thing passed as a parameter, it's okay to do this. If you were creating a new object inside the function (but on the stack, not literally using new), you'd have to return a value.

score 1 · Answer 4 · edited Mar 30 '12 at 08:44

Thought not sure what error message you are getting i have few suggestions

//Make the parameter as constant
           Abc(const Abc &source)
            {
                    this.a=source.a;
                    cout<<"copy constructor is called"<<endl;
            }
//change the return type
            ABC& operator = (const Abc &source)
            {
                  cout<<"overloaded assignment operator"<<endl;
                    if (this == &source)
                          return *this;
                    this.a=source.a;
                     **// return the existing object**
                   return *this;  
            }

Compilation error due to copy constructor

4 Answers4