0

I'm trying to implement a copy constructor and an overloaded assignment constructor. I've an Employee class that has three data members. 1-name 2-title 3-salary.

Employee::Employee(const Employee &emp)
{
    name = emp.name;
    title = emp.title;
    salary = emp.salary;
    cout << "\nOverloaded copy constructor called\n";
}
Employee Employee::operator = (const Employee &emp) //Overloading the assignment operator.
{
    name = emp.name;
    title = emp.title;
    salary = emp.salary;
    cout << "\nOverloaded assignment operator called\n";
    return emp;         //Or return *this.
}  

Here's what I do not understand:
1- I haven't got to the "this" pointer. Should my overloaded assignment operator return *this or the emp object. Because it seems to me that that object in the parameter is the right hand object at the assignment. So shouldn't I return the left hand object with *this(if that's what *this will be doing)?
2- At the main function, I tried to call to call the assignment operator first and the copy constructor after that. So I was expecting that I will see the cout statements I've included there one after the other however, Here's my output:

Overloaded copy constructor called
Overloaded assignment operator called
Overloaded copy constructor called
Overloaded copy constructor called

Why is this happening?

3-Do I have to pass the objects with const? The book I'm learning from does not.

In the main I just wrote

Employee x;
x = another;
Employee y = x;

"another" is just a (poorly named) Employee object I've initialized earlier in my code.
So shouldn't the first assignment output
"Overloaded assignment operator called" and the second assignment (Which isn't an assignment but a copy according to my understanding) output "Overloaded copy constructor called"

Mustafa
  • 129
  • 5
  • 15

4 Answers4

3

An assignment operator is normally implemented as

Employee& Employee::operator = (const Employee &emp)
{
    name = emp.name;
    title = emp.title;
    salary = emp.salary;
    cout << "\nOverloaded assignment operator called\n";
    return *this;      
}

note the difference in return type as a reference. You may find this question/answer to be useful Why must the copy assignment operator return a reference/const reference? . Without seeing your code that you refer to in your main() function we would only be guessing as to what is happening in your code.

Community
  • 1
  • 1
mathematician1975
  • 21,161
  • 6
  • 59
  • 101
0

1- I haven't got to the "this" pointer. Should my overloaded assignment operator return *this or the emp object. Because it seems to me that that object in the parameter is the right hand object at the assignment. So shouldn't I return the left hand object with *this(if that's what *this will be doing)?

You should return *this.

Note also that first you should check against self-assignment and return a reference:

Employee& Employee::operator = (const Employee &emp) //Overloading the assignment operator.
{
    if ( this != &emp)
    {
        name = emp.name;
        title = emp.title;
        salary = emp.salary;
    }
    cout << "\nOverloaded assignment operator called\n";
    return *this;
} 

Here you can find how to write assignment operator.

2- At the main function, I tried to call to call the assignment operator first and the copy constructor after that. So I was expecting that I will see the cout statements I've included there one after the other however

Please insert this code from main() so we can tell you more.

4pie0
  • 29,204
  • 9
  • 82
  • 118
  • I've seen this if statement in other questions here but I did not understand the need for it. What do you mean by self-assignment? – Mustafa Feb 07 '14 at 00:02
  • Employee e; e=e; Here this is explicit so looks unlikely however you can do something like this by mistake when you process objects in algorithms and structures. – 4pie0 Feb 07 '14 at 00:06
  • I've changed my assignment operator. But I've got an error when in the if statement (something about != does not match the operands) so I changed the if statement to if(this != &emp) and I returned *this. But my output now is Overloaded copy constructor called Overloaded assignment operator called Overloaded copy constructor called – Mustafa Feb 07 '14 at 00:25
  • In the main I just wrote Employee x; x = another; Employee y = x; "another" is just a (poorly named) Employee object I've initialized earlier in my code. So shouldn't the first assignment output "Overloaded assignment operator called" and the second assignment (Which isn't an assignment but a copy according to my understanding) output "Overloaded copy constructor called" – Mustafa Feb 07 '14 at 00:33
  • how did you create and initialize and pass to this place "another" object? first copy constructor is called probably for this object – 4pie0 Feb 07 '14 at 00:35
  • Oh dammit you are right. I've initialized my "another" object by copying it from a derived Engineer class. At the time of course I have not implemented my copy constructor. Good guess piotrus :). What about the const? All the question on stackoverflow have used the const in both but the book I'm using does not for either. – Mustafa Feb 07 '14 at 00:41
  • very well, I'm glad that it helped – 4pie0 Feb 07 '14 at 00:44
0

Edited : Let's copy constructor to do everything for you

Employee::Employee(const Employee &emp)
{
   if(this != &emp)
   {
       name = emp.name;
       title = emp.title;
       salary = emp.salary;
   }

  cout << "\nOverloaded copy constructor called\n";
}
Employee Employee::operator = (const Employee &emp) //Overloading the assignment   .
{       
   return emp;    //Having common assignment operations similar to copy constructor
}  
Varo
  • 831
  • 1
  • 7
  • 16
  • 1
    You are returning a reference to a local variable, which is bad as `temp` will be destructed and invalidate the reference. Your code also don't modify `this` nor return `*this` ( which is the default behavior of `operator=` ) – Tiago Gomes Feb 07 '14 at 00:31
0
  1. overloaded assignment operator should return *this pointer. You may implement a private method which would copy the class members and just call it from copy constructor/assignment operator, which would result in less duplicated code.

    void Employee::copy(const A& rhs) throw() 
    {
      name = rhs.name;
      title = rhs.title;
      salary = rhs.salary;
    }
    
    Employee::Employee(const Employee& rhs)
    {
      copy(rhs);
      //  cout << "copy constructor" << endl;
    }
    
    Employee& operator=(const Employee& rhs)
    {
      copy(rhs);
      //  cout << "assignment operator" << endl;
      return *this;
    }
    
  2. Your implementation of assignment operator does return the Employee object by value (it should be by reference), so as the effect the temporary objects are created by calling the copy c-tor.

    Overloaded copy constructor called // don't know where this printout comes from as I don't know what happens with 'another' object
    Overloaded assignment operator called // x = another; call
    Overloaded copy constructor called // the temporary object created by assignment operator's return statement (*this should be returned by reference not by value)
    Overloaded copy constructor called // Employee y = x; cal
    
  3. You should always pass the input parameters as const when you don't intend to modify them. I presume you ask about passing Employee objects as const references in assignment operator etc? - they should be const references

A bit more about exception safety and self assignment handling in assignment operator: GotW 59 and GotW 23

AdR
  • 115
  • 7