Operator overloading and parenthesis

[Guille]

Hi there,

I'm a new user of this forum and quite of a newbei in C/C++. I'd appreciate it if you could help me with one problem I'm having at the moment.

I've prepared a class to deal with matrices (yes, there are many of them around, but I found it a good exercise to learn all this staff). I've defined some methods with a * and + overloaded operators.

For example:

Matrix& Matrix::operator* (Matrix& B) {
    Matrix A = *this;
    Matrix temp(nRow,nCol);
    for (int i=0; i<nRow; i++)
        for (int j=0; j<nCol; j++)
            for (int n=0; n<nRow; n++)
                temp(i,j) += A(i,n)*B(n,j);
    return temp;
    }

Most of the times it seems to ok. These expressions are correctly calculated (being A, B and C any of these class element):

A + B;
A + B + C;
A * B + C;
etc.

But when I start to mess with parenthesis, it seems to ignore parts of the expressions:

(A * B) + (A * C) would give the same result as just (A * B).

Do you have any idea why is this happening? Any suggestions?

Thank you very much in advance,


Guille

[JosAH]

You are lucky the entire thing didn't explode or made daemons fly out of your nose
because you're returning a reference to a local variable (temp) which is a classic
nono. First define the operator*= which changes *this and returns a reference
to it. Define the operator* in terms of the operator*= and take Matrix type parameters
and return whatever operator*= returns.

kind regards,

Jos

[Guille]

You are lucky the entire thing didn't explode or made daemons fly out of your nose
because you're returning a reference to a local variable (temp) which is a classic
nono. First define the operator*= which changes *this and returns a reference
to it. Define the operator* in terms of the operator*= and take Matrix type parameters
and return whatever operator*= returns.

kind regards,

Jos

Hi JosAH,

Thanks for your reply, but I can't fully understand what do you mean. Could you express it in another way, or sketch it with some code?

Just in case it wasn't clear (I don't know if you suggest this), I DON'T want to modify the original matrix. I want the function to give back a new one, without modifying the old one.

Thanks once again,

Guille

[mac11]

Hi JosAH,

Thanks for your reply, but I can't fully understand what do you mean. Could you express it in another way, or sketch it with some code?

Just in case it wasn't clear (I don't know if you suggest this), I DON'T want to modify the original matrix. I want the function to give back a new one, without modifying the old one.

Thanks once again,

Guille

I'm not Jos, but the matrix you're returning is invalid after you return it - because you declared it in the local function - you can't return it (or a reference to it).

[Ganon11]

Jos is suggesting that you also overload the *= operator, which WOULD return its own Matrix. Then your * operator could simply call the *= operator on a copy of your Matrix and return it.

Alternatively, just don't make temp a local variable - try making it a dynamically allocated variable by using a pointer.

[boxfish]

Why not just return temp by value instead of by reference?

Matrix Matrix::operator* (Matrix& B) { // No &, see?
    // ...
    return temp;
}

By the way, it wouldn't hurt to have a const here and there:

Matrix Matrix::operator* (const Matrix& B) const {

the first one means that B won't be modified, the second means that this matrix won't be modified. They're ugly and they don't really do anything for your program, but they will give you an error if you modify something you're not supposed to.

I hope this works.

[Guille]

Ganon11 and JosAH:

Sorry, but I don't really catch it. I should overload the "*this", and that's done with somethig like this?

matrix matrix::operato r*=( ??? )

What should the parameter be? What should it return. Itself? With "return *this"?


Boxfish:

I had already tried to return temp itself. It works, and right, the array will remain, as the memory will not be freed after exiting the function. But when I concatenate more expressions in the same line like:

A*A + A*A

then I get an error like:

"error: no match for 'operator+' in '(&A)->Matrix::operat or*(((Matrix&)( &A))) + (&A)->Matrix::operat or*(((Matrix&)( &A)))'"

I know I could use intermediate variables in order to write simpler expressions. But I'd like to be able to do this kind of maths, and I find it a nice exercise in order to learn about this staff. Which obviously I still need, I appreaciate your patience O:)

[JosAH]

Ganon11 and JosAH:

Sorry, but I don't really catch it. I should overload the "*this", and that's done with somethig like this?

matrix matrix::operato r*=( ??? )

What should the parameter be? What should it return. Itself? With "return *this"?

Make it like this:

matrix& matrix::operator*=(matrix& rhs) {
   // multiply *this and rhs
   return *this;
}
matrix& matrix:: operator*(matrix& rhs) {
   matrix tmp= matrix(*this);
   return tmp*= rhs;
}

Note that the only copy we make is tmp= matrix(*this).

kind regards,

Jos

[Guille]

Make it like this:

matrix& matrix::operator*=(matrix& rhs) {
   // multiply *this and rhs
   return *this;
}
matrix& matrix:: operator*(matrix& rhs) {
   matrix tmp= matrix(*this);
   return tmp*= rhs;
}

Note that the only copy we make is tmp= matrix(*this).

kind regards,

Jos

Focusing in these two lines:

matrix& matrix::operator*=(matrix& rhs) {...}
...
{
    matrix tmp= matrix(*this);

What's calling the function (overloaded operator) defined in the first line, *this or matrix(*this)?

If it's not matrix(*this), what's matrix(*this) supposed to be? A constructor who copies the result obtained from *this into the newly created matrix?

One more point: if I execute the multiplication in the overloaded *this operator, I won't be able to use it with other methods which should make other operations (addition, multiplycation by a vector, inversion, etc). Right? How would the problem be solved with this other functions?

[Guille]

It finally seems to work, but it looks so easy now. I don't know why I messed up with references and so. Do you see any problem to the following code?

class Matrix {
private:
	double *data;

public:
	int nRow, nCol;
	Matrix (int, int);			   
	Matrix (int, int, string);
	double& operator() (int, int);
	Matrix operator+ (Matrix);	   
	Matrix operator* (Matrix);
	void   Matrix::print ();
}

...
// Constructors and access functions not shown
...

Matrix Matrix::operator+ (Matrix B)
{
    Matrix temp(nRow,nCol);
    for (int i=0; i<nRow; i++)
        for (int j=0; j<nCol; j++)
            temp(i,j) = (*this)(i,j) + B(i,j);
    return temp;
}

Matrix Matrix::operator* (Matrix B)
{
    Matrix temp(nRow,nCol);
    for (int i=0; i<nRow; i++)
        for (int j=0; j<nCol; j++)
            for (int n=0; n<nRow; n++)
            	temp(i,j) += (*this)(i,n)* B(n,j);
    return temp;
}

Thanks for your help and your patience! :)

Boxfish: I don't forget about the const's.

[Ganon11]

Does this work for two matrices of different sizes? For instance, it is valid to multiply a 3x4 matrix with a 4x2 matrix, but at first glance, it seems your code is making temp a 3x4 matrix. This is immediately wrong, as the resultant matrix should be a 3x2 matrix.

In general, when you multiply an IxJ matrix by a JxK matrix, the dimensions of the resultant matrix are IxK (the row number equals the row number of the first matrix, the column number equals the column number of the second matrix).

[Guille]

Does this work for two matrices of different sizes? For instance, it is valid to multiply a 3x4 matrix with a 4x2 matrix, but at first glance, it seems your code is making temp a 3x4 matrix. This is immediately wrong, as the resultant matrix should be a 3x2 matrix.

In general, when you multiply an IxJ matrix by a JxK matrix, the dimensions of the resultant matrix are IxK (the row number equals the row number of the first matrix, the column number equals the column number of the second matrix).

No, Ganon11, that code didn't work with matrixes of different sizes :) I've fixed that, thanks.

[Banfa]

matrix& matrix:: operator*(matrix& rhs) {
   matrix tmp= matrix(*this);
   return tmp*= rhs;
}

Surely this is returning a reference to an automatic (and therefore about to be deleted) object and likely to cause problems. Shouldn't this return matrix rather than matrix &?