What is the best way to avoid copying objects in an assignment operation?

[ChrisVer]

I have written the following class:

class Rectangle{
      private:
              double length;
              double height;
              const char* name;
      public:
             Rectangle (){};
             Rectangle (double l, double h, const char* n) : length(l), height(h), name(n) {};
             //copy constructor
             Rectangle ( const Rectangle & r) {
                       cout<<"Copying Rectangle Object : "<<endl;
                       length=r.length;
                       height=r.height;
                       };
             virtual ~Rectangle(){ cout<<"Rectangle object destroyed"<<endl; };

             //getters-setters            
             void setLength(double x) {length=x;};
             void setHeight(double x) {height=x;}; 
             double getLength() const {return length;};
             double getHeight() const {return height;};
         
             void Print() const{
                  cout<<"You have a Rectangle Object"<<endl;
                  cout<<"\tHeight : "<<height<<endl;
                  cout<<"\tLength : "<<length<<endl;   
              }
;
            
             //operator overloading
             Rectangle& operator=(Rectangle rhs){
                        //swap with copied rhs
                        std::swap(length, rhs.length);
                        std::swap(height, rhs.height);

                        return *this;
             };
};

int main(){
    Rectangle r(5.2,5.1);
    r.Print();

    Rectangle r8;
    r8 = r;
    r8=r8;
    r8.Print();
    system("PAUSE");
    return 0; 
}

When I run the code, I see something "weird": for each assignment operation I get a message:
Object copied
Object destroyed
After some investigation I found that the reason this happens is because in the assignment operator overload I pass the rhs as a value, so it's copied rhs, and when the assignment is over and gets out-of-scope the copy gets destroyed. I was wondering though, is it possible to avoid copying the object (can be slow)? I'm askinng because I read here that passing-by-reference is not exception safe...
http://www.cplusplus.com/articles/y8hv0pDG/
if not is it right to say that the operator = is doomed to be slower than copying the object as

Rectangle r8(r);

?

 

[Mark44]

I don't have time right now to take a detailed look at your code, but a couple of things jump out at me.
The first is a minor point, an assignment in main()

r8=r8;

Why are you doing this? Possibly the compiler will optimize this out, since nothing useful is happening.

Second, your overloaded assignment operator seems very inefficient, using swap() instead of just assigning the values from the two members of rhs to the corresponding members of what this points to. In addition, when you assign a value to a variable, you don't want to swap the values of the two expressions.

Here's a simple example I copied from the Microsoft documentation, https://msdn.microsoft.com/en-us/library/7ac5szsk.aspx

Point &Point::operator=( Point &ptRHS ) 
{ 
   _x = ptRHS._x; 
   _y = ptRHS._y; 
 
   return *this;  // Assignment operator returns left side. 
}

Third, your copy constructor needs to allocate memory for the copied object, using new(). What is apparently happening, and I need to look more closely into this, is that you are allocating a local variable on the stack, and this local variable gets deleted as soon as it is out of scope. If you create a Rectangle object on the heap, using new(), assign values to its members, and return the address of this memory, it won't get deleted as you're seeing in this code.

 

[SlowThinker]

When I run the code, I see something "weird": for each assignment operation I get a message:
Object copied
Object destroyed
After some investigation I found that the reason this happens is because in the assignment operator overload I pass the rhs as a value, so it's copied rhs, and when the assignment is over and gets out-of-scope the copy gets destroyed. I was wondering though, is it possible to avoid copying the object (can be slow)?

The code is specifically written to make a temporary copy, which is what makes it exception-safe.
IMHO the code only adds the two swaps and a call to destructor (which does nothing), otherwise it's as fast as a copy-constructor (because that's exactly what happens).
The class should be written in such a way that an empty object (Rectangle r8;) does not allocate any additional resources, otherwise

BloatedRectangle r9, r(2, 3);
r9=r;

would be slower than

BloatedRectangle r(2, 3);
BloatedRectangle r9(r);

because the former would allocate and free these additional resources.

 

[ChrisVer]

Why are you doing this? Possibly the compiler will optimize this out, since nothing useful is happening.

I added that expression because I wanted to try and avoid doing the assignment once the object on the rhs is the same as the one in the lhs, by adding something like an if( this != &rhs )... but I realized that with pass-by-value that's impossible because the copy will be given a random address, not the same as the original object.

Third, your copy constructor needs to allocate memory for the copied object, using new(). What is apparently happening, and I need to look more closely into this, is that you are allocating a local variable on the stack, and this local variable gets deleted as soon as it is out of scope. If you create a Rectangle object on the heap, using new(), assign values to its members, and return the address of this memory, it won't get deleted as you're seeing in this code.

hmm, I think the deletion in this case of the copied object makes sense... But are you suggesting this:

Rectangle ( const Rectangle & rec ) {
    Rectangle* tmp_r = new Rectangle();
    tmp_r ->length = rec.length;
    tmp_r->height = rec.height;
    //or just  tmp_r = &rec
    this = &tmp_r;
    //return *this;
}

in the copy constructor or in the assignment operator? Then I think I will fill up the memory by dummy copies of rectangles (if I don't expliciitly delete them myself).

 

[Mark44]

I added that expression because I wanted to try and avoid doing the assignment once the object on the rhs is the same as the one in the lhs, by adding something like an if( this != &rhs )... but I realized that with pass-by-value that's impossible because the copy will be given a random address, not the same as the original object.hmm, I think the deletion in this case of the copied object makes sense... But are you suggesting this:

Rectangle ( const Rectangle & rec ) {
    Rectangle* tmp_r = new Rectangle();
    tmp_r ->length = rec.length;
    tmp_r->height = rec.height;
    //or just  tmp_r = &rec
    this = &tmp_r;
    //return *this;
}

in the copy constructor or in the assignment operator? Then I think I will fill up the memory by dummy copies of rectangles (if I don't expliciitly delete them myself).

The example in my previous post was for an overloaded assignment operator, not a copy constructor. Regarding your example above, it starts off more or less as I was saying, but you can't use the this pointer as you did.
Two quotes from the MSDN page on the this pointer:

The this pointer is a pointer accessible only within the nonstatic member functions of a class, struct, or union type. It points to the object for which the member function is called.

Because the this pointer is nonmodifiable, assignments to this are not allowed. Earlier implementations of C++ allowed assignments to this.

 

[jtbell]

Third, your copy constructor needs to allocate memory for the copied object, using new().

No, I don't think so. As I recall, the copy constructor only needs to initialize the member data; memory has already been allocated for the object being initialized. If a data member is a pointer (as in the 'const char* name' here), and you want to do a "deep copy" of the object, then memory needs to be allocated for that pointer to point to.

ChrisVer's copy constructor looks OK to me, except that he hasn't initialized 'name'. Personally, I'd use a std::string instead of a char* for that. Then I could copy 'name' the same way as the other members:

class Rectangle {
    private:
        double length;
        double height;
        std::string name;
    public:
        //...other member functions omitted
        Rectangle (const Rectangle& r) {
            length = r.length;
            height = r.height;
            name = r.name;
            };
}

Or better, use an initializer list which invokes directly the copy constructors for the members:

    Rectangle (const Rectangle& r) :
        length (r.length), height (r.height), name (r.name)
        { };

Or better still, simply omit the copy constructor because the compiler will generate one that works exactly like my second version!

[added later] Aha, now I see you actually use an initializer list in your other constructor:

Rectangle (double l, double h, const char* n) : length(l), height(h), name(n) {}

Note that this doesn't copy the char array that 'n' points to. It only copies the pointer, so that you now have two pointers pointing to the same char array in memory. This may or may not be what you want. Are you sure the char array can't "disappear" on you unexpectedly? With my solution using std::string, the chars get copied. If you don't want the name of the rectangle to change after initialization, just make it const: 'const std::string name'. But then you can't initialize it via assignment as in my first version, only via the initializer list as in my second version.

 

[Mark44]

Third, your copy constructor needs to allocate memory for the copied object, using new().

No, I don't think so. As I recall, the copy constructor only needs to initialize the member data; memory has already been allocated for the object being initialized. If a data member is a pointer (as in the 'const char* name' here), and you want to do a "deep copy" of the object, then memory needs to be allocated for that pointer to point to.

Yes, you're right. For a "simple" class such as ChrisVer's Rectangle class, with no pointer members, dynamic memory allocation as I suggested is not needed at all. I'm a little rusty with C++, but as I'm teaching a class in this, I expect to dust off my skills.

 

[newjerseyrunner]

             //operator overloading
             Rectangle& operator=(Rectangle rhs){
                        //swap with copied rhs
                        std::swap(length, rhs.length);
                        std::swap(height, rhs.height);

                        return *this;
             };

This is not an assignment operator, at least not in the sense that you think it is. This is a function that creates a copy of the original parameter, then swaps the content with *this.

If for example you had this

Rectangle r(0,0,1,1);
Rectangle g;
g = r;

You expect that the rhs parameter will be r, right? Well, it won't be. Because you declared the parameter as an actual object, not a reference to an object, so a brand new one must be created.

 

[SlowThinker]

You expect that the rhs parameter will be r, right? Well, it won't be. Because you declared the parameter as an actual object, not a reference to an object, so a brand new one must be created.

This is explained in the article as a measure to ensure exception safety.
I try to avoid exceptions whenever I can, so I don't have much practice with them, but it looks like it might work.

 

[ChrisVer]

but as I'm teaching a class in this, I expect to dust off my skills.

we are in the same position on that... I'm in fact organizing the class tutorials and preparing the exercises, so I am trying to get used in C++ and writing things correctly or in alternative ways.

Note that this doesn't copy the char array that 'n' points to. It only copies the pointer, so that you now have two pointers pointing to the same char array in memory. This may or may not be what you want. Are you sure the char array can't "disappear" on you unexpectedly? With my solution using std::string, the chars get copied. If you don't want the name of the rectangle to change after initialization, just make it const: 'const std::string name'. But then you can't initialize it via assignment as in my first version, only via the initializer list as in my second version.

thanks for the input, I introduced that parameter to investigate [by printing the name of Rectangles] what was being coppied and destroyed in my assignment operator [though it didn't work at first]... I will try to make it work first with the string type and then with const char * (I want to use const char pointers because they are widely used in naming objects in ROOT classes... for example see the constructors here:
https://root.cern.ch/doc/master/classTH1F.html

 

[jtbell]

For learning purposes, using a const char* would give you some useful practice in resource management in classes. If you allocate memory in a class's constructors (e.g. for the name of your rectangle), you need to remember to deallocate it in the destructor.

std::string takes care of all this for you in its constructors and destructor, of course.