Thread: question about overloading operators

  1. #1
    Registered User
    Join Date
    Jul 2006
    Posts
    63

    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

  2. #2
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,412
    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.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  3. #3
    The larch
    Join Date
    May 2006
    Posts
    3,573
    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?
    I might be wrong.

    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
    Quoted more than 1000 times (I hope).

  4. #4
    Registered User
    Join Date
    Jul 2006
    Posts
    63
    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)
    Last edited by *DEAD*; 05-08-2008 at 01:12 AM.

  5. #5
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,412
    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.
    Last edited by laserlight; 05-08-2008 at 01:20 AM.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  6. #6
    Registered User
    Join Date
    Jul 2006
    Posts
    63
    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

  7. #7
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,412
    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.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  8. #8
    Registered User
    Join Date
    Jul 2006
    Posts
    63
    Quote Originally Posted by laserlight View Post
    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

  9. #9
    Registered User
    Join Date
    May 2008
    Location
    Paris
    Posts
    248
    Quote Originally Posted by *DEAD* View Post
    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
    };

  10. #10
    Registered User
    Join Date
    Jan 2005
    Posts
    7,366
    >> 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.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 25
    Last Post: 11-30-2008, 11:30 PM
  2. Newbie with C++ NOT, OR, AND operators question...
    By JiWiz in forum C++ Programming
    Replies: 4
    Last Post: 07-27-2008, 11:05 AM
  3. Replies: 2
    Last Post: 07-28-2006, 02:59 PM
  4. Replies: 9
    Last Post: 05-19-2006, 05:19 PM
  5. overloading operators..please help
    By Tozilla in forum C++ Programming
    Replies: 5
    Last Post: 03-29-2003, 11:32 PM