3

I know my copy constructors and overloaded operators are rather arbitrary in this case, but I looking for someone to verify if i'm making them correctly, as well as using *this correctly.

Also, could someone tell me why the copy constructor is called every time I return *this or an object of type Rectangle from any of my overloaded operators?

class Rectangle
{
    private:
        int length;
        int width;
        static int counter;
    public:
        Rectangle(int len = 0, int w = 0)
        {
            length = len;
            width = w;
            counter++;
        }
        ~Rectangle()
            { counter--; }
        static int getCounter()
            { return counter; }

        Rectangle(const Rectangle &);
        Rectangle operator+ (const Rectangle &);
        Rectangle operator= (const Rectangle &);
};

int Rectangle::counter = 0;

Rectangle::Rectangle(const Rectangle &right) : Rectangle()
{
    width = right.width;
    length = right.length;
}

Rectangle Rectangle::operator+ (const Rectangle &right)
{
    width += right.width;
    length += right.length;
    return *this;
}

Rectangle Rectangle::operator= (const Rectangle &right)
{
    width = right.width;
    length = right.length;
    return *this;
}
xaxxon
  • 19,189
  • 5
  • 50
  • 80
Eric Larson
  • 509
  • 3
  • 14

4 Answers4

5

Others have commented on how returning by value is invoking your copy constructor so I'm just going to answer the first part of the question:

if i'm making them correctly, as well as using *this correctly.

Your operator overloads are wrong:

Rectangle Rectangle::operator+ (const Rectangle &right)
{
    width += right.width;
    length += right.length;
    return *this;
}

this modifies *this and then returns a copy of it. That's not what you'd expect:

int b = 1, c = 2;
int a = b + c;

In the above, b and c are unmodified. A new value is produced and stored in a.

Rectangle Rectangle::operator+ (const Rectangle &right) const
{
    Rectangle newValue(*this);  // copy ctor temporary
    newValue.length += right.length;
    newValue.width += right.width;
    return newValue;
}

or if you've overloaded operator+= you could write it in terms of that:

Rectangle Rectangle::operator+ (const Rectangle &right) const
{
    Rectangle newValue(*this);  // copy ctor temporary
    newValue += right;
    return newValue;
}

The compiler will generally be able to perform Return Value Optimization and elide one of the copies that this code would otherwise produce.

Secondly:

Rectangle Rectangle::operator= (const Rectangle &right)
{
    width = right.width;
    length = right.length;
    return *this;
}

as you've learned, return-by-value returns a copy. operator= is supposed to return a reference to the current object:

Rectangle& Rectangle::operator= (const Rectangle& right)
{
    width = right.width;
    length = right.length;
    return *this;
}

Consider

a = b = c;

This executes

a.operator=(b.operator=(c));

returning a reference will save us a copy producing the parameter to a.operator=.

I'll finish with this suggestion for use of *this:

When you are returning a reference, *this says "return a reference to this concrete instance of my type". When you are returning a value and you say "*this" you say "return an rvalue (modifiable) instance of this class that looks like this", which invokes a copy.

kfsone
  • 23,617
  • 2
  • 42
  • 74
1

Copy constructor is getting called as you are returning the object by value which needs to be copied at caller side.

Rectangle Rectangle::operator= (const Rectangle &right)
{
    width = right.width;
    length = right.length;
    return *this;     <<<<<<<< copy constructor call.
}

You can avoid this copy if you return that object by reference.

Rectangle& Rectangle::operator= (const Rectangle &right)
{
    width = right.width;
    length = right.length;
    return *this;     <<<<<<<<<<<< No copy constructor call.
}
ravi
  • 10,994
  • 1
  • 18
  • 36
-1

Also, could someone tell me why the copy constructor is called every time I return *this or an object of type Rectangle from any of my overloaded operators?

Your operators return Rectangle objects which have to be constructed based on the value of the Rectangle stored in *this. Creating a Rectangle from a Rectangle is what a copy constructor does.

If you want to return the this Rectangle itself (instead of a copy), you'll want to return a reference to a Rectangle -- so Rectangle&

Rectangle & Rectangle::operator+ (const Rectangle &right)
xaxxon
  • 19,189
  • 5
  • 50
  • 80
  • It's a "solution" if you don't want the copy constructor to be called. Though the implementation you have in operator+ is better suited for operator+=, since it modifies the operand. – xaxxon May 02 '16 at 05:49
  • `operator+` shouldn't return a reference... it was correct as posted in the question, returning by value. (`+=` would return a reference to `*this`) – Tony Delroy May 02 '16 at 05:51
  • @TonyD He asked why the copy constructor was being called. His code is terribly wrong in many fashions, but this is the reason his copy constructor is being called -- which was his specific question. I specifically said *IF* he wants to return *this, then he should return a ref. I didn't suggest his "business logic" was correct -- and in the question he specifies that the code is arbitrary. – xaxxon May 02 '16 at 05:52
  • @TonyD "I know my copy constructors and overloaded operators are rather arbitrary in this case" – xaxxon May 02 '16 at 05:54
  • Is it perfectly fine for classes to always call the copy constructor when returning objects from overloading operator functions? I feel like that would present some problem at some point. – Eric Larson May 02 '16 at 05:54
  • @Matimio it's not incorrect, but it's often not the behavior you want, and it comes with the performance cost of creating a new object. There are more advanced techniques which mitigate some of that cost (move constructor/move assignment), but that's not really relevant here. – xaxxon May 02 '16 at 05:55
  • Disregarding performance, would my class work properly? I'm just unsure if my most basic knowledge is incorrect. – Eric Larson May 02 '16 at 06:00
  • In your code, when you say Rectangle foo = a + b; `a` is getting changed, foo now has the same value as `a`, but if you change `a`, `foo` doesn't change. If you change `foo`, then `a` doesn't change. If you return a reference, changing one will change the other. It simply depends what behavior you want. For +, usually you don't change either `a` or `b` and return a new (copy constructed) element. for +=, you would change `a` and return a reference to it. (`a` becomes `this` inside your operator+) – xaxxon May 02 '16 at 06:03
  • So how would I properly implement Rectangle foo = a + b, where a and b stay the same, while only changing foo? – Eric Larson May 02 '16 at 06:08
  • @TonyD 's answer gives you some suggestions to that follow up question. – xaxxon May 02 '16 at 06:13
-2

overloaded operators are rather arbitrary in this case, but I looking for someone to verify if i'm making them correctly, as well as using *this correctly.

Nope - not quite correct.

Rectangle Rectangle::operator= (const Rectangle &right) is usually written to return a Rectangle&, so other operations can be made on the assigned-to object, it can be passed as a function argument etc..

Rectangle Rectangle::operator+(const Rectangle &right) is implemented incorrectly. It's common to first implement += and then use that in the implementation of +, but if you want to write it independently it could be:

Rectangle Rectangle::operator+(Rectangle right) const
{
    right.width += width;
    right.length += length;
    return right;
}

That way, the right is parameter is by-value, so changes to it don't affect the caller, and you can modify it and return it by value. This might seem inefficient, but the by-value return is typically optimised away (it's called a Return Value Optimisation - RVO).

The other approach I mentioned looks like this...

Rectangle& operator+=(const Rectangle& rhs)
{
     length += rhs.length;
     width += rhs.width;
     return *this;
}

Rectangle operator+(Rectangle rhs) const
{
     return (rhs += *this);
}

That said, if there are any implicit constructors for your type, it's necessary to make + a non-member operator, so the left-hand-side argument gets a change to be implicitly constructed:

Rectangle operator+(const Rectangle& lhs, const Rectangle& rhs)
{
    return Rectangle(lhs.length + rhs.length, lhs.width + rhs.width);
}

Also, could someone tell me why the copy constructor is called every time I return *this or an object of type Rectangle from any of my overloaded operators?

In the code in your question, the return type is Rectangle (not by reference), and you return *this: what that does is make a copy of the object on which the function's invoked and returns that copy. The copy constructor is used to create the copy.

Tony Delroy
  • 102,968
  • 15
  • 177
  • 252
  • 1
    You didn't answer his question. – xaxxon May 02 '16 at 05:54
  • It's worth maybe a mention for RVO. – skypjack May 02 '16 at 05:55
  • @xaxxon: I had addressed the first question (well, more literally a request *"looking for someone to verify"*) in the body of Matimio's post before your comment, and have had time since to address the second question (also asked in the title). Cheers. – Tony Delroy May 02 '16 at 07:20