By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
455,431 Members | 1,754 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 455,431 IT Pros & Developers. It's quick & easy.

Operator overloading and parenthesis

P: 6
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:

Expand|Select|Wrap|Line Numbers
  1. Matrix& Matrix::operator* (Matrix& B) {
  2.     Matrix A = *this;
  3.     Matrix temp(nRow,nCol);
  4.     for (int i=0; i<nRow; i++)
  5.         for (int j=0; j<nCol; j++)
  6.             for (int n=0; n<nRow; n++)
  7.                 temp(i,j) += A(i,n)*B(n,j);
  8.     return temp;
  9.     }
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
Nov 5 '08 #1
Share this Question
Share on Google+
12 Replies


Expert 10K+
P: 11,448
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
Nov 5 '08 #2

P: 6
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
Nov 5 '08 #3

100+
P: 256
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).
Nov 5 '08 #4

Ganon11
Expert 2.5K+
P: 3,652
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.
Nov 5 '08 #5

boxfish
Expert 100+
P: 469
Why not just return temp by value instead of by reference?
Expand|Select|Wrap|Line Numbers
  1. Matrix Matrix::operator* (Matrix& B) { // No &, see?
  2.     // ...
  3.     return temp;
  4. }
By the way, it wouldn't hurt to have a const here and there:
Expand|Select|Wrap|Line Numbers
  1. 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.
Nov 5 '08 #6

P: 6
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::operator*=( ??? )

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::operator*(((Matrix&)(&A))) + (&A)->Matrix::operator*(((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:)
Nov 6 '08 #7

Expert 10K+
P: 11,448
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::operator*=( ??? )

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

Expand|Select|Wrap|Line Numbers
  1. matrix& matrix::operator*=(matrix& rhs) {
  2.    // multiply *this and rhs
  3.    return *this;
  4. }
  5. matrix& matrix:: operator*(matrix& rhs) {
  6.    matrix tmp= matrix(*this);
  7.    return tmp*= rhs;
  8. }
  9.  
Note that the only copy we make is tmp= matrix(*this).

kind regards,

Jos
Nov 6 '08 #8

P: 6
Make it like this:

Expand|Select|Wrap|Line Numbers
  1. matrix& matrix::operator*=(matrix& rhs) {
  2.    // multiply *this and rhs
  3.    return *this;
  4. }
  5. matrix& matrix:: operator*(matrix& rhs) {
  6.    matrix tmp= matrix(*this);
  7.    return tmp*= rhs;
  8. }
  9.  
Note that the only copy we make is tmp= matrix(*this).

kind regards,

Jos

Focusing in these two lines:

Expand|Select|Wrap|Line Numbers
  1. matrix& matrix::operator*=(matrix& rhs) {...}
  2. ...
  3. {
  4.     matrix tmp= matrix(*this);
  5.  
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?
Nov 6 '08 #9

P: 6
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?

Expand|Select|Wrap|Line Numbers
  1. class Matrix {
  2. private:
  3.     double *data;
  4.  
  5. public:
  6.     int nRow, nCol;
  7.     Matrix (int, int);               
  8.     Matrix (int, int, string);
  9.     double& operator() (int, int);
  10.     Matrix operator+ (Matrix);       
  11.     Matrix operator* (Matrix);
  12.     void   Matrix::print ();
  13. }
  14.  
  15. ...
  16. // Constructors and access functions not shown
  17. ...
  18.  
  19. Matrix Matrix::operator+ (Matrix B)
  20. {
  21.     Matrix temp(nRow,nCol);
  22.     for (int i=0; i<nRow; i++)
  23.         for (int j=0; j<nCol; j++)
  24.             temp(i,j) = (*this)(i,j) + B(i,j);
  25.     return temp;
  26. }
  27.  
  28. Matrix Matrix::operator* (Matrix B)
  29. {
  30.     Matrix temp(nRow,nCol);
  31.     for (int i=0; i<nRow; i++)
  32.         for (int j=0; j<nCol; j++)
  33.             for (int n=0; n<nRow; n++)
  34.                 temp(i,j) += (*this)(i,n)* B(n,j);
  35.     return temp;
  36. }
Thanks for your help and your patience! :)

Boxfish: I don't forget about the const's.
Nov 6 '08 #10

Ganon11
Expert 2.5K+
P: 3,652
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).
Nov 7 '08 #11

P: 6
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.
Nov 10 '08 #12

Banfa
Expert Mod 5K+
P: 8,916
Expand|Select|Wrap|Line Numbers
  1. matrix& matrix:: operator*(matrix& rhs) {
  2.    matrix tmp= matrix(*this);
  3.    return tmp*= rhs;
  4. }
  5.  
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 &?
Nov 10 '08 #13

Post your reply

Sign in to post your reply or Sign up for a free account.