0

I have a simple operator + overload method for a class Macierz which is supposed to get the 2d matrix variable from the object, and sum all of this matrix variable with the same field of another Macierz object.

Macierz & operator + (Macierz &f) {
    Macierz newM;

    for (int i = 0; i < length; i++)
        for (int j = 0; j < length; j++) 
            newM.macierz[i][j] = macierz[i][j] + f.macierz[i][j];


    return newM;
}

The calculating works fine. Displaying the newM variable produces a correct value, however when I try to use it like this:

float y[3][3] = { {1.00f, 2.00f, 3.00f}, { 4.00f, 5.00f, 6.00f }, { 7.00f, 8.00f, 9.00f } };
Macierz m5(y); //fill m5 array with values from above
    Macierz m2(2.00f); //fill m2 entirely with 2.00f values

    cout << m2 + m5;

The result is an array of zeroes.

How could this happen? The macierz variable is a simple float[3][3] field.

Any help would be amazing

aln447
  • 981
  • 2
  • 15
  • 44
  • You + operator returns a reference to a local variable. https://stackoverflow.com/questions/4643713/c-returning-reference-to-local-variable – Jeffery Thomas Apr 18 '18 at 18:38

1 Answers1

5

The result is an array of zeroes.

You are returning a reference to a function local variable. The reference becomes a dangling reference as soon as the function returns. As a consequence, your program has undefined behavior.

Change the function to return by value instead.

// Remove the reference type specifier
//     |
//     v 
Macierz operator + (Macierz &f) {
    Macierz newM;
    ...
    return newM;
}

I suggest couple of further refinements to the function.

  1. Change it to a const member function.
  2. Change the argument type to a const&.

With those changes, you can use the function using const objects too.

Macierz operator + (Macierz const& f) const {
    Macierz newM;
    ...
    return newM;
}

A related post: Is it okay to use an overloaded operator to implement another operator overload?.

R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • Worked! Thank you! – aln447 Apr 18 '18 at 16:59
  • This is a whole lotta unnecessary overhead. The temporary here is not needed. Just add to the original matrix and return the reference to *this* at the end – SomeWittyUsername Apr 18 '18 at 17:01
  • @SomeWittyUsername, it is idiomatic to return an object by value from the binary `+` operator function. Think of `int a = 10 + 20;` if the `operator+` function returns a reference, you can't use that. – R Sahu Apr 18 '18 at 17:03
  • @SomeWittyUsername -- maybe, but that's a different operation than the original. The original doesn't modify the object that `+` is called on; your version does. In fact, your version would typically be named `operator+=`, and would be used to implement `operator+(const Macierz&, const Macierz&)`. – Pete Becker Apr 18 '18 at 17:04
  • @RSahu Yes, I meant what Pete Becker wrote below. – SomeWittyUsername Apr 18 '18 at 17:06