[ACCEPTED]-Check for "self-assignment" in copy constructor?-copy-constructor

Personally, I think your professor is wrong 24 and here's why.

Sure, the code will compile. And 23 sure, the code is broken. But that's as 22 far as your Prof has gone with his reasoning, and 21 he then concludes "oh well we should see 20 if we're self-assigning and if we are, just 19 return."

But that is bad, for the same reason 18 why having a global catch-all catch(...) which does 17 nothing is Evil. You're preventing an immediate 16 problem, but the problem still exists. The 15 code is invalid. You shouldn't be calling 14 a constructor with a pointer to self. The 13 solution isn't to ignore the problem and 12 carry on. The solution is to fix the code. The 11 best thing that could happen is your code will 10 crash immediately. The worst thing is that the 9 code will continue in an invalid state for 8 some period of time and then either crash 7 later (when the call stack will do you no 6 good), or generate invalid output.

No, your 5 professor is wrong. Do the assignment without 4 checking for self-assignment. Find the 3 defect in a code review or let the code 2 crash and find it in a debug session. But 1 don't just carry on as if nothing has happened.

This is valid C++ and calls the copy constructor:

Test a = a;

But 2 it makes no sense, because a is used before 1 it's initialized.

If you want to be paranoid, then:

class Test
{
   Test(const Test &copy)
   {
       assert(this != &copy);
       // ...
   }
};

You never 5 want to continue if this == &copy. I've never bothered 4 with this check. The error doesn't seem 3 to frequently occur in the code I work with. However 2 if your experience is different then the 1 assert may well be worth it.

Your instructor is probably trying to avoid 3 this situtation -

#include <iostream>

class foo
{
    public:
    foo( const foo& temp )
    {
        if( this != &temp )
        std::cout << "Copy constructor \n";
    }
};

int main()
{
    foo obj(obj);  // This is any how meaning less because to construct
                   // "obj", the statement is passing "obj" itself as the argument

}

Since the name ( i.e., obj ) is 2 visible at the time declaration, the code 1 compiles and is valid.

Your instructor may be thinking of the check 5 for self-assignment in the copy assignment operator.

Checking 4 for self-assignment in the assignment operator 3 is recommended, in both Sutter and Alexandrescu's 2 "C++ Coding Standards," and Scott Meyer's 1 earlier "Effective C++."

In normal situations, it seems like here 8 is no need to. But consider the following 7 situation:

class A{ 
   char *name ; 
public:
   A & operator=(const A & rhs);
};

A & A::operator=(const A &rhs){
   name = (char *) malloc(strlen(rhs.name)+1);
   if(name) 
      strcpy(name,rhs.name);
   return *this;
}

Obviously the code above has an 6 issue in the case when we are doing self 5 assignment. Before we can copy the content, the 4 pointer to the original data will be lost 3 since they both refer to same pointer. And 2 that is why we need to check for self assignment. Function 1 should be like

A & A::operator=(const A &rhs){
     if(this != &rhs){
       name = (char *) malloc(strlen(rhs.name)+1);
       if(name) 
          strcpy(name,rhs.name);
     } 
   return *this;
}

When writing assignment operators and copy 10 constructors, always do this:

struct MyClass
{
    MyClass(const MyClass& x)
    {
        // Implement copy constructor. Don't check for
        // self assignment, since &x is never equal to *this.
    }

    void swap(MyClass& x) throw()
    {
        // Implement lightweight swap. Swap pointers, not contents.
    }

    MyClass& operator=(MyClass x)
    {
        x.swap(*this); return *this;
    }
};

When passing x by 9 value to the assignment operator, a copy 8 is made. Then you swap it with *this, and let 7 x's destructor be called at return, with 6 the old value of *this. Simple, elegant, exception safe, no 5 code duplication, and no need for self assignment 4 testing.

If you don't know yet about exceptions, you 3 may want to remember this idiom when learning 2 exception safety (and ignore the throw() specifier 1 for swap for now).

I agree that self check doesn't make any 15 sense in copy constructor since object isn't 14 yet created but your professor is right 13 about adding the check just to avoid any 12 further issue. I tried with/without self 11 check and got unexpected result when no 10 self check and runtime error if self check 9 exists.

class Test
{    
    **public:**
    Test(const Test& obj )
    {
       size = obj.size;
       a = new int[size];   
    }

    ~Test()
    {....}

    void display()
    {
        cout<<"Constructor is valid"<<endl;
    }
    **private:**

 }

When created copy constructor and 8 called member function i didn

 Test t2(t2);
 t2.display(); 

Output:
Inside default 7 constructor
Inside parameterized constructor
Inside 6 copy constructor
Constructor is valid

This 5 may be correct syntactically but doesn't 4 look right. With self check I got runtime 3 error pointing the error in code so to avoid 2 such situation.

    Test(const Test& obj )
    {
        if(this != &obj )
        {
            size = obj.size;
            a = new int[size];
        }
    }

Runtime Error:
Error in `/home/bot/1eb372c9a09bb3f6c19a27e8de801811': munmap_chunk(): invalid 1 pointer: 0x0000000000400dc0

Generally, the operator= and copy constructor 5 calls a copy function, so it is possible 4 that self-assignment occurs. So,

Test a;
a = a;

For example,

const Test& copy(const Test& that) {
    if (this == &that) {
        return *this
    }
    //otherwise construct new object and copy over
}
Test(const &that) {
    copy(that);
}

Test& operator=(const Test& that) {
    if (this != &that) { //not checking self 
        this->~Test();
    }
    copy(that);
 }

Above, when 3 a = a is executed, the operator overload 2 is called, which calls the copy function, which 1 then detects the self assignment.

Writing copy-assignment operators that are 20 safe for self-assignment is in the C++ core guidelines and 19 for a good reason. Running into a self-assignment 18 situation by accident is much easier than 17 some of the sarcastic comments here suggest, e.g. when 16 iterating over STL containers without giving 15 it much thought:

std::vector<Test> tests;
tests.push_back(Test());
tests.resize(10);
for(int i = 0; i < 10; i++)
{
  tests[i] = tests[0]; // self when i==0
}

Does this code make sense? Not 14 really. Can it be easily written through 13 carelessness or in a slightly more complex 12 situation? For sure. Is it wrong? Not really, but 11 even if so... Should the punishment be a 10 segfault and a program crash? Heck no.

To 9 build robust classes that do not segfault 8 for stupid reasons, you must test for self-assignment 7 whenever you don't use the copy-and-swap 6 idiom or some other safe alternative. One 5 should probably opt for copy-and-swap anyway 4 but sometimes it makes sense not to, performance 3 wise. It is also a good idea to know the 2 self-assignment test pattern since it shows 1 in tons of legacy code.