40

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 ?

2
  • Take a look at the copy and swap idiom Commented Apr 9, 2012 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. Commented Mar 23, 2015 at 23:10

6 Answers 6

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.

Sign up to request clarification or add additional context in comments.

5 Comments

Actually the standard way is Copy & Swap method not the one mentioned.
@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).
@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.
@Als I was refering to returning by reference, not the implementation. I will clarify this.
@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).
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)
}

2 Comments

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.
I was referring to returning a copy by value in your first example.
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.

Comments

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();  
}

Comments

0

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

Comments

-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

}

1 Comment

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.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.