Thread: copy constructor and operator =

  1. #1
    Advanced Novice linucksrox's Avatar
    Join Date
    Apr 2004
    Location
    Michigan
    Posts
    198

    copy constructor and operator =

    I've got a program that I believe uses correct syntax, however in Visual studio 2005 it compiles fine, and in dev-cpp 4.9.9.2 it gives me an error. my code is this:
    Code:
    // main.cpp
    //proper includes...
    
    int main()
    {
       Matrix m1(3,3), m2(3,3), m3;
       Matrix m4(2,3);
       m3 = m1 + m2;
       return 0;
    }
    *******************************
    // Matrix.h
    // proper includes
    
    // MatrixException class
    class Matrix
    {
    // private stuff
    public:
    // constructor destructor and everything else
    // i'm having trouble with these 2 things:
        Matrix(Matrix &m);
        void operator = (Matrix &m);   
    };
    
    ************************************
    // Matrix.cpp
    // proper includes
    
    // all functions defined properly, except these two:
    
    Matrix::Matrix(Matrix &m)
    {
        numRows = m.numRows;
        numCols = m.numCols;
        data = new int[numRows*numCols];
        for (int i=0; i < numRows*numCols ; i++)
            data[i] = m.data[i];
    }
    
    void Matrix::operator = (Matrix &m)
    {
        numRows = m.numRows;
        numCols = m.numCols;
        data = new int[numRows*numCols];
        for (int i=0; i < numRows*numCols ; i++)
            data[i] = m.data[i];
    }
    now everything works fine except in main, where i say
    Code:
    m3 = m1 + m2
    dev's compiler tells me
    Code:
    ...main.cpp no match for 'operator=' in 'm3 = Matrix::operator+(Matrix&)(((Matrix&)(&m2)))'
    note D:\School\Semester 3\CPS271\Matrix Class Lab\Matrix.h:50 candidates are: void Matrix::operator=(Matrix&)
    but that is my only error. any suggestions?
    "What are all you parallelograms doing here?" - Peter Griffin (to Joe and his wheelchair buddies)

  2. #2
    Registered User
    Join Date
    Jan 2005
    Posts
    7,366
    Try making the parameter to your operator= a const Matrix&, since you shouldn't be modifying the passed in value anyway. You will have to make any other methods you use in operator= const correct (you should already be making all your methods const correct).

  3. #3
    Registered User
    Join Date
    Apr 2003
    Posts
    2,663
    Post your operator+() function. It's producing something that's not a Matrix&.

  4. #4
    Registered User
    Join Date
    Nov 2005
    Posts
    85
    copy constructors should check whether they are not copying to itself same with assignment operators.

    So check if(this == &whatever) return *this because otherwise no copying is required

  5. #5
    Advanced Novice linucksrox's Avatar
    Join Date
    Apr 2004
    Location
    Michigan
    Posts
    198
    my operator+ function is defined like this:
    Code:
    Matrix Matrix::operator+(Matrix & m)
    {
        if (numRows != m.numRows)
            throw MatrixException("Addition requires same # of rows ", __FILE__, __LINE__);
        if (numCols != m.numCols)
            throw MatrixException("Addition requires same # of columns ", __FILE__, __LINE__);
        Matrix retMatrix(numRows, numCols);
        for (int i=0; i < numRows; i++)
            for (int j=0; j < numCols; j++)
            {
                int value = get(i,j) + m.get(i,j);
                retMatrix.put(value, i, j);
            }
        return retMatrix;
    }
    EDIT: oh sorry guys, i posted that code just before I tried switching my parameters correctly to const when the function does not change the parameter passed in. now everything works correctly, no warnings or errors, and my program runs correctly. Thanks Daved especially, and everyone else for your input.
    Last edited by linucksrox; 11-28-2005 at 11:15 PM.
    "What are all you parallelograms doing here?" - Peter Griffin (to Joe and his wheelchair buddies)

  6. #6
    Registered User
    Join Date
    Jan 2005
    Posts
    7,366
    >> copy constructors should check whether they are not copying to itself same with assignment operators.

    In general this is not required, and so it wouldn't be causing compile errors. It is also not necessary in cases where self-assignment is rare and operator= will work anyway.

    In this case, though, jlf029 is right that it probably would be beneficial since you are using dynamically allocated memory (you don't want to delete your old data if it is also the new data).

    In addition, your assignment operator should delete the existing data, or your code will create a memory leak every time operator= is used.

  7. #7
    Registered User
    Join Date
    Apr 2003
    Posts
    2,663
    Try making the parameter to your operator= a const Matrix&
    Why does that have any effect? Within the operator=() function, the parameter m is not used to call a const member function, so as far as I can tell, whether you make it const or not should have no effect on whether the function executes error free.

    linucksrox,

    Your operator=() should not return void if you want to be able to write:

    m1 = m2 = m3;

    which is equivalent to:

    m1 = (m2 = m3);

    Being able to do that is generally provided for when overloading operators that can be chained together like that. In the case of operator=(), you want to return a reference to the left hand side, which will be: *this
    Last edited by 7stud; 11-29-2005 at 01:20 AM.

  8. #8
    the hat of redundancy hat nvoigt's Avatar
    Join Date
    Aug 2001
    Location
    Hannover, Germany
    Posts
    3,130
    Why does that have any effect? Within the operator=() function, the parameter m is not used to call a const member function, so as far as I can tell, whether you make it const or not should have no effect on whether the function executes error free.
    The other way round... no non-const member function is called, so making it a const reference will save you a lot of trouble. You cannot pass a const Matrix if the operator only takes a non-const reference. And he doesn't need a non-const reference, so it should be const.

    Code:
    CLASS& CLASS::operator = ( const CLASS& other );
    hth
    -nv

    She was so Blonde, she spent 20 minutes looking at the orange juice can because it said "Concentrate."

    When in doubt, read the FAQ.
    Then ask a smart question.

  9. #9
    Registered User
    Join Date
    Apr 2003
    Posts
    2,663
    The other way round...
    Yeah, I screwed that up. Non-const member's can call both regular and const member functions.


    It still don't get why the compiler flagged the non-const argument as an error.
    You cannot pass a const Matrix if the operator only takes a non-const reference.
    As far as I can tell, the op didn't:
    Code:
    Matrix m1(3,3), m2(3,3), m3;
       Matrix m4(2,3);
       m3 = m1 + m2;
    Code:
    Matrix Matrix::operator+(Matrix & m)
    Last edited by 7stud; 11-29-2005 at 02:32 AM.

  10. #10
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    The cause of the original problem is not related to const-ness of arguments or methods (although use of const in the places people identified is a good idea).

    The cause is that Matrix::operator=() returns void. Make it return a reference to Matrix (and return *this) instead.
    Last edited by grumpy; 11-29-2005 at 04:25 AM. Reason: Disable smilies

  11. #11
    Registered User
    Join Date
    Apr 2003
    Posts
    2,663
    Hi grumpy,

    Thanks for taking a look.

    Quote Originally Posted by grumpy
    The cause of the original problem is not related to const-ness of arguments or methods (although use of const in the places people identified is a good idea).

    The cause is that Matrix:: operator=() returns void. Make it return a reference to Matrix (and return *this) instead.
    I don't see how that can be correct. Returning void from operator=() prevents you from writing chain statements like this:

    m1=m2=m3;

    Here is an example of returning void that doesn't cause a compiler error(in VC++6):
    Code:
    class Apple
    {
    public:
    	void operator=(Apple& a)
    	{}
    
    	Apple operator+(Apple& a)
    	{
    		Apple x;
                    return x;
    	}
    };
    
    int main()
    {
     
    	Apple a, b, c;
    	
    	c = a + b;
    
    	return 0;
    }
    Are you saying the dev C++ compiler enforces the chaining idiom?

    The error seems to indicate that the compiler cannot find a matching operator= function to execute:
    no match for 'operator='
    The only way that could be the case is if the argument is a const object; and the only way the argument could be a const object is if operator+ returns a const object. However, as far as I can tell, when the operator+() returns-by-value the copy constructor is called, and it creates a non-const object, so I don't get it.

    Can anyone verify that the original code caused a dev c++ error?
    Last edited by 7stud; 11-29-2005 at 05:46 AM.

  12. #12
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    Quote Originally Posted by 7stud
    I don't see how that can be correct. Returning void from operator=() prevents you from writing chain statements like this:
    I stand corrected.

    Quote Originally Posted by 7stud
    The error seems to indicate that the compiler cannot find a matching operator= function to execute:

    The only way that could be the case is if the argument is a const object; and the only way the argument could be a const object is if operator+ returns a const object. However, as far as I can tell, when the operator+() returns-by-value the copy constructor is called, and it creates a non-const object, so I don't get it.
    I'd have to dig through the standard to be sure, but your comments here have triggered a recollection of a discussion I saw by some gurus a couple of years back. IIRC, in your example,
    Code:
    m3 = m1 + m2;
    the result of the addition (returned from operator+()) is allowed to be stored in a temporary, and that temporary is passed by reference to operator=()
    Code:
       Matrix temporary(m1+m2);
       m3.operator=(temporary);
    The problem is that, if the compiler does this, the temporary is an rvalue, so can only be bound to a const reference. Hence an error message --- even if the compiler is subsequently smart enough to optimise the temporary out of existance.

Popular pages Recent additions subscribe to a feed