Operator overloading c++ advice

Question

I am struggling to get my head around operator overloading. In this case the + operator I have and example of what I have tried, any help would be greatly appreciated.

I am getting an error which says "invaild use of 'class Matrix' I am unsure on how to fix this how can I add these two Matrix objects together?

Matrix Matrix::operator+(const Matrix& rhs){
return Matrix(Matrix + rhs.Matrix());
}


   Matrix::Matrix(int MM, int NN){
                  M = MM;
                  N = NN;
                  data = new double[M * N];
                  for ( int i =0; i < M; i++)
                  {
                      for (int j = 0; j < N; j++)
                      {
                          data[i* N+j] = (double) 1000 + i*N+j;
                    //      cout << data[i*N+j] <<"\t";
                      }
                      //cout <<"\n";
                  }

       cout << "Matrix Constructor... (code to be implemented here!!!)\n";}

Thanks

a good read about operator overloading http://courses.cms.caltech.edu/cs11/material/cpp/donnie/cpp-ops.html — Marius, Apr 06 '12 at 12:31

jpm · Answer 1 · 2012-04-06T11:08:49.387

4

rhs is a Matrix
calling a constructor like a method is very illegal
in Matrix + rhs, Matrix is not an identifier
Once you get your identifiers straightened out, *this + rhs is equivalent to this->operator+(rhs). It should be obvious from this that all you've done here is create an infinite recursion.

edited Apr 06 '12 at 11:08

answered Apr 06 '12 at 10:55

jpm

3,165
17
24

1

I didn't even look at your constructor code, so please don't interpret its absence from this list my endorsement of its validity. – jpm Apr 06 '12 at 10:59
1

Also, since you are managing the `data` member via heap memory by using `new` in your constructor (and presumably a corresponding `delete`in your destructor), you should define a copy constructor and assignment operator that does a proper deep copy of the data, as the default ones C++ provides can lead to problems like multiple `Matrix` instances referencing the same `data` pointer (if copied) and dangling pointer problems on copies. – Preet Kukreti Apr 06 '12 at 11:17
`Matrix` is an identifier, it just names a type and not a variable. – CB Bailey Apr 06 '12 at 12:49
Point taken, and I'm all for precision and accuracy. Still, I'm sure you know what I meant. :) – jpm Apr 06 '12 at 13:14

MPelletier · Accepted Answer · 2012-04-17T12:43:53.110

jpm's answer is very important to look at. Once you've fixed these things, you can look at mine.

Essentially, an operator overload is no different than any other function.

So, in the case of:

Matrix Matrix::operator+(const Matrix& rhs)

What you're really doing there is saying: add rhs to the current matrix, and return a new matrix. Your overload should not alter the current matrix. Help yourself and use a constant:

Matrix Matrix::operator+(const Matrix& rhs) const

A matrix addition like that should first check if the matrices have the same dimensions so that you can add them together, then loop through all "cells" and add them, and create a matrix out of that. For that, I'm guessing you'll need a second constructor, something like:

Matrix::Matrix(int MM, int NN, double values[])
{
    M = MM;//TODO: change to width
    N = NN;//TODO: change to height
    data = new double[M*N];
    for(int i < 0; i < M; i++)
        for(int j < 0; j < N; j++)
            data[i * N+j] = values[i * N+j];
}

Just to clarify a little. When he says "add rhs to my current matrix," he doesn't me alter the values of the current matrix. This operator should not affect the state of the current matrix. Instead, we want to take the values from this matrix, add them to the values of rhs, and cram all the results into a brand new matrix. Just to be clear. — jpm, Apr 06 '12 at 11:12

score 0 · Answer 3 · edited May 23 '17 at 12:20

return Matrix(Matrix + rhs.Matrix());
              ^^^^^^       ^^^^^^

You're using a type name (Matrix) where there should be an expression - that's what the compiler is complaining about. You're also trying to call the constructor on an existing object, which is invalid (and nonsensical). You also seem to be trying to implement operator+ in such a way that it calls itself; if you do somehow make it compile, then it will cause a stack overflow due to infinite recursion.

Perhaps the easiest way to implement addition is to implement operator+= to add one matrix to an existing one, and then implement operator+ in terms of that:

Matrix & Matrix::operator+=(Matrix const & rhs) {
    // Perform addition here
    for (int i = 0; i < N*M; ++i) {
        data[i] += rhs.data[i];
    }
    return *this;
}

// This can be a non-member function
// Pass "lhs" by value to get a copy, then modify and return that.
Matrix operator+(Matrix lhs, Matrix const & lhs) {
    return lhs += rhs;
}

// Or if you really want a member function for some reason
Matrix Matrix::operator+(Matix const & rhs) const {
    return Matrix(*this) += rhs;
}

This does require you to have a correct copy constructor - since you're allocating memory yourself in the constructor, and presumably deallocating it in the destructor, you must implement a copy-constructor to correctly allocate new memory (per the Rule of Three), otherwise you'll get a double-deletion after copying a matrix.

Operator overloading c++ advice

3 Answers3