CODEWITHSUNDEEP
c++variable-assignmentoperator-keywordmultiplication
stollgrin
stollgrin

Reputation: 109

Strange assignment / multiplication operator behaviour in C++

Could somebody please explain me wth is wrong with my operators:

Matrix3D Matrix3D::operator*(Matrix3D& m) {
    Matrix3D ret;
    for(int i=0;i<4;i++) {
        for(int j=0;j<4;j++) {
            ret._data[i][j]=0.0;
            for(int k=0;k<4;k++) {
                ret._data[i][j] += (this->_data[i][k]*m._data[k][j]);
            }
        }
    }
    return ret;
}

Matrix3D& Matrix3D::operator=(Matrix3D& m) {
    if(this==&m) {
        return *this;
    }
    for(int i=0;i<4;i++) {
        for(int j=0;j<4;j++) {
            this->_data[i][j] = m._data[i][j];
        }
    }
    return *this;
}

Matrix3D Matrix3D::Rotation(double ax, double ay, double az) {
    Matrix3D rotX;
    Matrix3D rotY;
    Matrix3D rotZ;
    rotX(
        1,          0,          0,          0,
        0,          cos(ax),    -sin(ax),   0,
        0,          sin(ax),    cos(ax),    0,
        0,          0,          0,          1
    );
    rotY(
        cos(ay),    0,          sin(ay),    0,
        0,          1,          0,          0,
        -sin(ay),   0,          cos(ay),    0,
        0,          0,          0,          1
    );
    rotZ(
        cos(az),    -sin(az),   0,          0,
        sin(az),    cos(az),    0,          0,
        0,          0,          1,          0,
        0,          0,          0,          1
    );

    // Matrix3D ret;
    // ret = rotX*rotY*rotZ;
    // This doesn't work
    // C:\(...)\Matrix3D.cpp|100|error: no match for 'operator=' in 'ret = Matrix3D::operator*(Matrix3D&)(((Matrix3D&)(& rotZ)))'|

    // however this does work

    Matrix3D ret = rotX*rotY*rotZ;
    return ret;
}

as stated in the code above, something like

    Matrix3D ret;
    ret = rotX*rotY*rotZ;

will result in 

    C:\(...)\Matrix3D.cpp|100|error: no match for 'operator=' in 'ret = Matrix3D::operator*(Matrix3D&)(((Matrix3D&)(& rotZ)))'|

compilation error, while something like

    Matrix3D ret = rotX*rotY*rotZ;

will compile without any warning or error whatsoever (don't know if the matrixes are right tho, haven't checked that yet...).

Upvotes: 2

Views: 332

Answers (4)

Filip Ros&#233;en
Filip Ros&#233;en

Reputation: 63872

Proper declarations of both operator* and operator=:

Matrix3D  Matrix3D::operator* (Matrix3D const& m) const;
Matrix3D& Matrix3D::operator= (Matrix3D const& m);

Reading material:

Your operator= accepts a Matrix3D& though rotX*rotY*rotX will result in an rvalue and therefor you cannot bind a reference to it.

To make work your operator= should accept a Matrix3D const&, which can be bound to both lvalues and rvalues.

Even though it may look like Obj a = b uses Obj::operator= it's not, it'll use what is normally referred to as the copy-constructor. Read more about it here.

Upvotes: 5

kennytm
kennytm

Reputation: 523604

Actually, both operators are declared wrongly. They should take a const reference to allow rvalues be accepted. 

Matrix3D Matrix3D::operator*(const Matrix3D& m) const
//                           ^^^^^              ^^^^^
//                                              this too, since you're not
//                                              changing the LHS during '*'.

Matrix3D& Matrix3D::operator=(const Matrix3D& m)
//                            ^^^^^

You'll see that * doesn't work with rotX*(rotY*rotZ).

The reason Matrix3D ret = rotX*rotY*rotZ; compiles is because the operator= is not invoked at all. It just calls the copy constructor of Matrix3D to the result of the multiplication. And the multiplication works because rotX * rotY * rotZ is rewritten to

rotX.operator*(rotY).operator*(rotZ)

and rotY and rotZ are both lvalues, so they can be bound to Matrix3D&.

Upvotes: 8

CygnusX1
CygnusX1

Reputation: 21818

Matrix3D& Matrix3D::operator=(Matrix3D& m)

You request that the right-side parameter is an l-value. It must be an existing variable, cannot be a temporary value. Consider using const Matrix3D& m instead.

The Matrix3D ret = rotX*rotY*rotZ; works because it is not using operator= at all, instead it uses the default constructor Matrix3D::Matrix3D(const Matrix3D&).

Upvotes: 1

Kerrek SB
Kerrek SB

Reputation: 477434

Change the signature of the assignment operator to be const-correct:

Matrix3D& Matrix3D::operator=(Matrix3D const & m)
//                                     ^^^^^

In your example code, the assignment operator wants to bind to a temporary, and temporaries cannot bind to non-constant references. By contrast, the copy constructor, which is invoked by the second piece of code, does accept a const reference, so there's no error.

Upvotes: 4

Related Questions