39

I have used the following code for assignment operator overloading:

SimpleCircle SimpleCircle::operator=(const SimpleCircle & rhs)
{
     if(this == &rhs)
        return *this;
     itsRadius = rhs.getRadius();
     return *this;
}

My Copy Constructor is this:

SimpleCircle::SimpleCircle(const SimpleCircle & rhs)
{
    itsRadius = rhs.getRadius();
}

In the above operator overloading code, copy constructor is called as there is a new object is being created; hence I used the below code:

SimpleCircle & SimpleCircle::operator=(const SimpleCircle & rhs)
{
    if(this == &rhs)
       return *this;
    itsRadius = rhs.getRadius();
    return *this;
}

Its working perfectly and the copy constructor problem is avoided, but is there any unknown issues (to me) regarding this ?

kaushik
  • 2,308
  • 6
  • 35
  • 50
  • Take a look at the [copy and swap idiom](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) – Praetorian Apr 09 '12 at 16:37
  • 1
    @Praetorian The copy and swap idiom is good if you know an item can throw during the assignment operator or if you are working with software you haven't developed and don't know if a throw will occur. If you are working with your own items and you know there will be no throws using the copy and swap idiom is not necessary. – Zachary Kraus Mar 23 '15 at 23:10

6 Answers6

21

There are no problems with the second version of the assignment operator. In fact, that is the standard way for an assignment operator.

Edit: Note that I am referring to the return type of the assignment operator, not to the implementation itself. As has been pointed out in comments, the implementation itself is another issue. See here.

juanchopanza
  • 223,364
  • 34
  • 402
  • 480
  • 4
    Actually the standard way is *Copy & Swap* method not the one mentioned. – Alok Save Apr 09 '12 at 16:33
  • 1
    @Als: Copy and swap is certainly standard when you need to deal with remote ownership. When dealing with a single, simple value I'd call it overkill (though still probably better than what's in the question). – Jerry Coffin Apr 09 '12 at 16:35
  • @JerryCoffin: Actually, I do advocate Copy and Swap for single valued call because it is hard to mess up copy and swap once you've learnt to do it the right way. – Alok Save Apr 09 '12 at 16:37
  • 1
    @Als I was refering to returning by reference, not the implementation. I will clarify this. – juanchopanza Apr 09 '12 at 16:38
  • @Als: What I'd generally advocate is that if it contains only simple values, the default implementation will normally work (and should normally be used). – Jerry Coffin Apr 09 '12 at 16:42
8

The second is pretty standard. You often prefer to return a reference from an assignment operator so that statements like a = b = c; resolve as expected. I can't think of any cases where I would want to return a copy from assignment.

One thing to note is that if you aren't needing a deep copy it's sometimes considered best to use the implicit copy constructor and assignment operator generated by the compiler than roll your own. Really up to you though ...

Edit:

Here's some basic calls:

SimpleCircle x; // default constructor
SimpleCircle y(x); // copy constructor
x = y; // assignment operator

Now say we had the first version of your assignment operator:

SimpleCircle SimpleCircle::operator=(const SimpleCircle & rhs)
{
     if(this == &rhs)
        return *this; // calls copy constructor SimpleCircle(*this)
     itsRadius = rhs.getRadius(); // copy member
     return *this; // calls copy constructor
}

It calls the copy constructor and passes a reference to this in order to construct the copy to be returned. Now in the second example we avoid the copy by just returning a reference to this

SimpleCircle & SimpleCircle::operator=(const SimpleCircle & rhs)
{
    if(this == &rhs)
       return *this; // return reference to this (no copy)
    itsRadius = rhs.getRadius(); // copy member
    return *this; // return reference to this (no copy)
}
AJG85
  • 15,849
  • 13
  • 42
  • 50
  • Actually I wanted to make sure where this copy constructor is being called. I am using a cout<<"I am being called"; in that. Only using that causing problems. Its not copying values. – kaushik Apr 09 '12 at 16:41
  • I was referring to returning a copy by value in your first example. – AJG85 Apr 09 '12 at 16:46
7

Under the circumstances, you're almost certainly better off skipping the check for self-assignment -- when you're only assigning one member that seems to be a simple type (probably a double), it's generally faster to do that assignment than avoid it, so you'd end up with:

SimpleCircle & SimpleCircle::operator=(const SimpleCircle & rhs)
{
    itsRadius = rhs.getRadius(); // or just `itsRadius = rhs.itsRadius;`
    return *this;
}

I realize that many older and/or lower quality books advise checking for self assignment. At least in my experience, however, it's sufficiently rare that you're better off without it (and if the operator depends on it for correctness, it's almost certainly not exception safe).

As an aside, I'd note that to define a circle, you generally need a center and a radius, and when you copy or assign, you want to copy/assign both.

Jerry Coffin
  • 476,176
  • 80
  • 629
  • 1,111
1
#include<iostream>

using namespace std;

class employee
{
    int idnum;
    double salary;
    public:
        employee(){}

        employee(int a,int b)
        {
            idnum=a;
            salary=b;
        }

        void dis()
        {
            cout<<"1st emp:"<<endl<<"idnum="<<idnum<<endl<<"salary="<<salary<<endl<<endl;
        }

        void operator=(employee &emp)
        {
            idnum=emp.idnum;
            salary=emp.salary;
        }

        void show()
        {
            cout<<"2nd emp:"<<endl<<"idnum="<<idnum<<endl<<"salary="<<salary<<endl;
        }
};

main()
{
    int a;
    double b;

    cout<<"enter id num and salary"<<endl;
    cin>>a>>b;
    employee e1(a,b);
    e1.dis();
    employee e2;
    e2=e1;
    e2.show();  
}
Martin Gal
  • 16,640
  • 5
  • 21
  • 39
0

it's right way to use operator overloading now you get your object by reference avoiding value copying.

-1

this might be helpful:

// Operator overloading in C++
//assignment operator overloading
#include<iostream>
using namespace std;

class Employee
{
private:
int idNum;
double salary;
public:
Employee ( ) {
    idNum = 0, salary = 0.0;
}

void setValues (int a, int b);
void operator= (Employee &emp );

};

void Employee::setValues ( int idN , int sal )
{

salary = sal; idNum = idN;

}

void Employee::operator = (Employee &emp)  // Assignment operator overloading function
{
salary = emp.salary;
}

int main ( )
{

Employee emp1;
emp1.setValues(10,33);
Employee emp2;
emp2 = emp1; // emp2 is calling object using assignment operator

}
  • 5
    It can be of more help if you can explain the key areas in the code snippet posted, than just posting a code snippet just like that. – RinoTom Sep 24 '13 at 14:46