Error passing by value in operator overloading

Question

I'm trying to understand why passing by reference works and passing by value does not in a simple class definition

#include <iostream>

class Rettangolo {
  int a, b;
  public:
    Rettangolo (Rettangolo& r) {
      a = r.a;
    }
    Rettangolo (int _a, int _b) : a(_a), b(_b) {};
    Rettangolo operator+= (Rettangolo r1);
    void print() {
      std::cout << a << ", " << b;
    }
};

Rettangolo Rettangolo::operator+= (Rettangolo r1) {
  a += r1.a;
  b += r1.b;
  return *this;
};

int main() {
  std::cout << "Hello World!\n";
  Rettangolo recta (1, 2);
  Rettangolo rectb (5, 6);
  recta += rectb;
  recta.print ();
}

If I use (Rettangolo r1) in the parameter of operator+= as in the example above, the output is "6,2". But if I use (Rettangolo& r1) in the parameter the output is "6, 8" as I would expect

Fyi, `operator +=` should return a reference; not a value copy as yours does. Just saying. — WhozCraig, Oct 19 '20 at 22:07
This code contains multiple issues. You should read on copy constructors and operator overloading. — mfnx, Oct 19 '20 at 22:15

score 4 · Accepted Answer · answered Oct 19 '20 at 22:06

This is because your copy constructor is not fully functional. It only copies a, and not b. Therefore when rectb is copied when you call operator+=, passing rectb by value, the a is copied and added to recta but the b is not copied and therefore not added to recta. To fix this, add the b copying in the copy constructor, or just delete it, and the compiler will generate a default copy constructor for you that copies both a and b.

score 1 · Answer 2 · answered Oct 19 '20 at 22:57

There are several issues with your code.

The most important issue is that your copy constructor is not copying b at all. When you pass a Rettangolo by value to operator+=, the compiler has to make a copy of it, so inside of your operator+= the copied r1.b value has an indeterminate value, which in your case just happens to be 0, but that is not guaranteed. Passing r1 by reference instead skips your broken copy constructor.

Also, the copy constructor and operator+= need to take a Rettangolo by const reference.

Also, your operator+= is not returning *this by reference, so it is returning another copy (which you are ignoring).

Try this instead:

#include <iostream>

class Rettangolo {
  int a, b;
  public:
    Rettangolo (int _a, int _b) : a(_a), b(_b) {}

    Rettangolo (const Rettangolo& r) : a(r.a), b(r.b) {}
    // or: Rettangolo (const Rettangolo &r) = default;
    // or: simply omit this constructor and let the compiler generate it for you!

    Rettangolo& operator+= (const Rettangolo &r1);

    void print() const {
      std::cout << a << ", " << b;
    }
};

Rettangolo& Rettangolo::operator+= (const Rettangolo &r1) {
  a += r1.a;
  b += r1.b;
  return *this;
};

int main() {
  std::cout << "Hello World!\n";
  Rettangolo recta (1, 2);
  Rettangolo rectb (5, 6);
  recta += rectb;
  recta.print ();
}

Error passing by value in operator overloading

2 Answers2