question about overloading operators

hi im trying to make a matrix class which makes use of operator overloading. Ive followed several tutorials, and the one which looked best implemented the binary operators (+ - etc) using unary operators (+=) as such.

Code:

#include "Matrix.h"

Matrix::Matrix(int m, int n, float* values)
{
    nn = n;
    mm = m;
    matrix = new float[mm*nn];
    for(int mi = 0; mi < mm; mi++)
    {
        for(int ni = 0; ni < nn; ni++)
        {
            matrix[mi*nn+ni] = values[mi*nn+ni];
        }
    }
}

Matrix::Matrix()
{
    nn = 1;
    mm = 1;
    matrix = new float[1];
    matrix[0] = 0;
}

Matrix& Matrix::operator=(const Matrix &m2)
{
    if(&m2 != this)
    {
        nn = m2.nn;
        mm = m2.mm;
        delete[] matrix;
        matrix = new float[mm*nn];
        for(int mi = 0; mi < mm; mi++)
        {
            for(int ni = 0; ni < nn; ni++)
            {
                matrix[mi*nn+ni] = m2.matrix[mi*nn + ni];
            }
        }
    }
    return *this;
}

const Matrix& Matrix::operator+=(const Matrix &m2)
{
    for(int mi = 0; mi < mm; mi++)
    {
        for(int ni = 0; ni < nn; ni++)
        {
            matrix[mi*nn+ni] += m2.matrix[mi*nn + ni];
        }
    }
    wxString output;
    output.Printf(_T("%f %f\n%f %f"), matrix[0], matrix[1], matrix[2], matrix[3]);
    wxMessageBox(output);
    return *this;
}

const Matrix& Matrix::operator+(const Matrix &m2)
{
    Matrix result = *this;
    result += m2;
    return result;
}

the problem is in these examples result is created on the stack (i assume) and i get warning messages whenever i compile. the obvious solution would be to return a Matrix on the heap and write another constructor which accepts a Matrix pointer assign it to itself using the = operator, but this seems messy. whats the standard approach to this problem
thx

That's because you are returning a reference to the local variable result in operator+. This is a Bad Thing. Instead you should return by value. I suggest that you provide operator+ as a free function:

Code:

Matrix operator+(const Matrix &m1, const Matrix &m2)
{
    return Matrix(m1) += m2;
}

If operator+ was a member function as in your example, it should be declared const.

In operator + you cannot return a reference. You can return references to objects that already exist (such as *this in +=) but operator+ doesn't work on this and is supposed to supply a new object.
If you are going to use the heap, hell will break loose and you'll have memory leaks all over the place. For example, what about use cases that rely on temporaries:

Code:

a = b + c + d;

Who would clean up the temporaries which are not stored anywhere?

yes i have no doubt all of hells minions would converge on me if i were to use the heap. The problem remains tho i have to return a new Matrix (which is currently created by Matrix result = *this) so it can be used by the equals operator.

as for
a = b + c + d
i think that is equivalent to
(a = (b + (c + d)))
so then lets say the first operation takes place and a temp var e = c + d
then when e is assigned operator= deletes the return of operator+(c, d)

The problem remains tho i have to return a new Matrix (which is currently created by Matrix result = *this) so it can be used by the equals operator.

What you call the "equals operator" operator is more commonly called the copy assignment operator. However, the copy assignment operator is not invoked in the line below:

Code:

Matrix result = *this;

rather, the copy constructor is invoked.

so then lets say the first operation takes place and a temp var e = c + d
then when e is assigned operator= deletes the return of operator+(c, d)

Even if everything else is implemented correctly, your operator+ is still wrong. It is not a problem with managing memory, it is a problem with returning a reference to a local variable. The local variable does not exist as soon as control leaves operator+, thus:

Code:

e = c + d;

means that e is a reference to an object that no longer exists. If you return by value instead, then it would be correct.

yes but even with a copy constructor

Code:

Matrix::Matrix(Matrix &copy)
{
    nn = copy.nn;
    mm = copy.mm;
    matrix = new float[mm*nn];
    for(int mi = 0; mi < mm; mi++)
    {
        for(int ni = 0; ni < nn; ni++)
        {
            matrix[mi*nn+ni] = copy.matrix[mi*nn + ni];
        }
    }
}

const Matrix& Matrix::operator+(const Matrix &m2)
{
    Matrix result(*this);
    result += m2;
    return result;
}

result is still a local variable residing on the stack

yes but even with a copy constructor

Both of these use the copy constructor:

Code:

Matrix result1(*this);
Matrix result2 = *this;

This uses the copy assignment operator:

Code:

result1 = result;

result is still a local variable residing on the stack

Eh, local variables reside on the stack (depending on the definition of "stack"). That's not the problem. The problem is that you are returning a reference to a local variable (this is the third time I have told you this).

The solution is to change your operator+'s return type from const Matrix& to Matrix:

Code:

Matrix Matrix::operator+(const Matrix &m2) const
{
    Matrix result(*this);
    result += m2;
    return result;
}

I have taken the liberty of making your member operator+ const correct by defining it as const.

Originally Posted by laserlight

Both of these use the copy constructor:

Code:

Matrix result1(*this);
Matrix result2 = *this;

This uses the copy assignment operator:

Code:

result1 = result;

Eh, local variables reside on the stack (depending on the definition of "stack"). That's not the problem. The problem is that you are returning a reference to a local variable (this is the third time I have told you this).

The solution is to change your operator+'s return type from const Matrix& to Matrix:

Code:

Matrix Matrix::operator+(const Matrix &m2) const
{
    Matrix result(*this);
    result += m2;
    return result;
}

I have taken the liberty of making your member operator+ const correct by defining it as const.

sorry i guess i missed the point twice lol
neway that worked and thx a million for all the help

Originally Posted by *DEAD*

yes but even with a copy constructor

Code:

Matrix::Matrix(Matrix &copy)
{
    nn = copy.nn;
    mm = copy.mm;
    matrix = new float[mm*nn];
    for(int mi = 0; mi < mm; mi++)
    {
        for(int ni = 0; ni < nn; ni++)
        {
            matrix[mi*nn+ni] = copy.matrix[mi*nn + ni];
        }
    }
}

const Matrix& Matrix::operator+(const Matrix &m2)
{
    Matrix result(*this);
    result += m2;
    return result;
}

result is still a local variable residing on the stack

If you have implemented the copy constructor as you described, the compiler won't recognize it as such? a copy-constructor always has a 'const&' argument:

Code:

class foo
{
public:
foo( void); // default constructor declaration
foo( foo const&); // copy constructor declaration
};

>> a copy-constructor always has a 'const&' argument
This actually isn't true. See auto_ptr for an example of a non-const reference parameter for a copy constructor.

That said, in almost all cases you should have a reference to const because you rarely want to modify the source object when copying from it.