Copy constructor problemm..Help

[L0s3r]

class mymatrix{

 private:
   int** arr;
   int m ,n;

 public:

   mymatrix()
   {
       std::cout<<"\nConstructor entered.";
       std::cout<<"\nEnter no. of rows of the matrix:";
       std::cin>>m;

       std::cout<<"\nEnter no. of columns of the matrix:";
       std::cin>>n;

       arr = new int*[m];

       for(int i=0;i<m;i++)
       {
           arr[i]=new int[n];
       }

       std::cout<<"\nConstructor exited.";
   }


/****************************************************************************************/
   mymatrix (mymatrix& t)
   {
       m=t.m;
       n=t.n;

       std::cout<<"\nCopy constructor has been called...";
       for(int i=0;i<m;i++)
       {
           for(int j=0;j<n;j++)
           {
               arr[i][j]=t.arr[i][j];
           }
       }
   }

/********************************************************************************************/

   ~mymatrix()
   {
        std::cout<<"\nDestuctoer entered.";
        for(int i=0;i<m;i++)
        {
            delete [] arr[i];
        }
        delete arr;
        std::cout<<"\nDestructor exited.";
   }

   void getmatrix();

   mymatrix operator +(const mymatrix &);

   mymatrix operator *(const mymatrix &);

   bool operator ==(const mymatrix &);

   void display();

};


mymatrix mymatrix :: operator +(const mymatrix & m1)
{
    if(m1.m==m && m1.n==n)
    {
        std::cout<<"\nMatrix can be added togather";
    }
    else
    {
        std::cout<<"\nMatrix can't be add togather...!!!";
        exit(0);
    }

    mymatrix t;

    for(int i=0;i<m;i++)
    {
        for(int j=0;j<n;j++)
        {
            t.arr[i][j] = m1.arr[i][j] + arr[i][j];
        }
    }


    return t;
}

void mymatrix :: getmatrix()
{
   for(int i=0;i<m;i++)
   {
       for(int j=0;j<n;j++)
       {
           std::cout<<"\nEnter value for arr["<<i<<"]["<<j<<"]:::";
           std::cin>>arr[i][j];
           std::cout<<"\n";
       }
   }

}

bool mymatrix :: operator ==(const mymatrix & m1)
{
    if(m==m1.m && n==m1.n)
    {
        std::cout<<"\nMatrix can be compared togather";
    }
    else
    {
        std::cout<<"\nMatrix can't be compared togather...!!!";
        exit(0);
    }



    for(int i=0;i<m;i++)
    {
        for(int j=0;j<n;j++)
        {
            if( arr[i][j] != m1.arr[i][j])
            {
                return false;
            }
        }
    }

    return true;
}

void mymatrix :: display()
{
   for(int i=0;i<m;i++)
   {
       std::cout<<"\n";
       for(int j=0;j<n;j++)
       {
           std::cout<<arr[i][j]<<"\t";
       }
   }
}

mymatrix mymatrix :: operator *(const mymatrix & m1)
{
    if(m1.m==n && m1.n==m)
    {
        std::cout<<"\nMatrix can be multiplied togather";
    }
    else
    {
        std::cout<<"\nMatrix can't be multiplied togather...!!!";
        exit(0);
    }

    mymatrix t;

    for(int i=0;i<m;i++)
    {
        for(int j=0;j<m1.n;j++)
        {
            for(int k=0;k<m1.m;k++)
            {
                t.arr[i][j] = arr[i][k]  *  m1.arr[k][j];
            }
        }
    }

    return t;
}

Hello all .

I'm having problem with my copy constructor. When I'm returning object from operator+ func. . No copy constructor is called. Whereas it should be.

#include<iostream>
#include<stdlib.h>
#include"mymatrix.h"

int main(void)
{
    mymatrix c1;

    c1.getmatrix();
    c1.display();

    mymatrix c2;
    c2.getmatrix();
    c2.display();

    mymatrix c3;
    c3 = c1 + c2;
    c3.display();

    mymatrix c4;
    c4 = c1 * c2;
    c4.display();

    return 0;

}

[Narue]

No copy constructor is called. Whereas it should be.

Actually, it shouldn't be. You're trying to use the copy assignment operator, not the copy constructor. A use of the copy constructor would be mymatrix c3 = c1 + c2; . Generally, if you need a copy constructor, you also need a copy assignment operator and destructor.

[hag++]

I am a little fuzzy on this subject but I believe the problem lies in the fact that the copy constructor is not always called by default when using the assignment operator on your objects. Your best best is to either manually override the assignment operator. Feel free to correct me if I'm wrong

EDIT:
Narue beat me to it.

[mike_2000_17]

There is a special allowance for compilers to optimize away the copy constructor when returning an object by value from a function. Basically, the compiler sees that the returned object is assigned to a variable and thus will use that variable directly, within the function, to create the returned object. This avoids useless copying. Never depend on the fact that the copy-constructor will be called once or twice during a return-by-value and direct assignment at the call-site.

BTW, the copy-constructor should take a const reference. You also need an assignment operator (preferably implemented as a copy-and-swap idiom).

[L0s3r]

I read that a copy constructor will always be called while returning objects by value.ANd so it should in this problem.

Besides I created another program. In this too I have the same problem.

#include<iostream>
#include<string.h>

class test
{
    char* p;

    public:

    test()
    {
        p=NULL;
    }

    test(test& t)
    {
        std::cout<<"\nCopy constructor called...";
        p=new char[strlen(t.p)];
        strcpy(p,t.p);
    }
    ~test(){
    delete[] p;
    }

    void getstr()
    {
        char arr[256];
        std::cout<<"\nEnter you name:";
        std::cin>>arr;
        p=new char[strlen(arr)];
        strcpy(p,arr);
            }

    test operator =(test& t)
    {
        test t1;
        std::cout<<"\nAssignment operator called...";
        t1.p=new char[strlen(t.p)];
        strcpy(t1.p,t.p);
        return t1;

    }

    void display()
    {
        std::cout<<"\nYour name is:"<<p;
    }

};


int main(void)
{
    test t1;
    t1.getstr();
    t1.display();

    test t2 =t1;
    t2.display();

    test t3;
    t3=t1;
    t3.display();

    return 0;
}

[mike_2000_17]

The code you posted outputs the following on my computer:

Enter you name:sdfs

Your name is:sdfs
Copy constructor called...
Your name is:sdfs
Assignment operator called...
Your name is:

The above is exactly what I would expect. Where is the trouble? Besides the fact that your code has memory leaks and corruptions, and an improper assignment operator.

Here is a bit of explanation of what is going on:

int main(void)
{
    test t1;      //calls the default constructor to create t1.
    t1.getstr();
    t1.display();

    test t2 =t1;  //could call the default constructor and then the assignment operator.
    t2.display(); // but will almost certainly just call the copy-constructor.

    test t3;      //calls the default constructor to create t3.
    t3=t1;        //calls the assignment operator on t3, and the 
    t3.display(); // return value is discarded, no copy-constructor is called.

    return 0;
}

Now, for the bugs in your code.

1) The strlen() function does not count the null-termination character. So, you need to add one character when allocating memory to hold a copy of a string. As such:

p = new char[strlen(t.p) + 1];

2) Be const-correct. Your copy-constructor should not be modifying the object passed to it, so it should be a const reference. As so:

test(const test& t)  //notice const here.
    {
        std::cout<<"\nCopy constructor called...";
        p=new char[strlen(t.p) + 1];   //notice + 1 here.
        strcpy(p,t.p);
    }

3) The assignment operator should, in theory, also take a const reference. But, a more practical solution is to take a copy (i.e. by value) and then swap (hence, the name copy-and-swap). And also, you should return by reference or const-reference depending on what you want. As so:

test& operator=(test t) //notice, pass-by-value (i.e. makes a copy)
{
    using std::swap; //import std-scoped swap function overloads.
    swap(p,t.p);     //swap the pointer in t with the one in 'this' (which will be destroyed automatically).
    return *this;    //return a (const-)reference to 'this'.
}

**Notice how you don't have to repeat the (error-prone) "new char[]" allocation and copy procedure when you implement the copy-and-swap idiom instead.

4) Printing a C-string which is actually a NULL pointer will put the output stream in an error state. If there is a possibility that your string pointer is NULL, you need to check that condition before printing:

void display() const    //notice the const here, because 'this' is not modified.
    {
        if(p)
            std::cout << "\nYour name is:" << p;
        else
            std::cout << "\nYou have no name!";
    }

Get used to using RAII (Resource Acquisition Is Initialization), and using RAII classes, like std::string instead of C-style strings.

[L0s3r]

That was quite an effort.I appreciate it, thanks. :)

But can you provide soln. when I have to call the assignment operator by value and it should also return by value.

My qs. is while returning by value it would make a copy of the "to be returned temp variable". Wouldn't it do so by calling a copy constructor.
Edit: it is not calling during return t1.

Also, when I passed a test object to assignment operator by value, a copy constructor should be called there.Isn't it?
Edit: It is calling it.

If not then provide me a link to a book , where it is written so.

[mike_2000_17]

>>My qs. is while returning by value it would make a copy of the "to be returned temp variable". Wouldn't it do so by calling a copy constructor.

This is called the Return Value Optimization. This is classic. Do not, ever, depend on copy-constructors being called. This means, don't do operations in the copy-constructor that don't pertain to "just copying the object". Obviously, the C++ compilers have some liberty in optimizing away some copy-construction and assignments when it involves temporaries, for obvious reasons. The side-effect that this has is that you have to keep your copy-constructors simple enough that simple copy-semantics still hold (i.e. the copy-constructor should not have any other extra side-effects).

>>Also, when I passed a test object to assignment operator by value, a copy constructor should be called there.Isn't it?

Yes, that's the whole point. If you are going to make a copy of an object that is passed as a parameter to a function, you should pass-by-value and you get a copy of the passed object for free (and with some performance benefits as well).

>>it should also return by value.

I don't see any purpose for returning a value from the assignment operator (at least, not in this context). But if you really want to do that, fine, do it. I'm pretty sure it's not necessary, and certainly wasteful.

[L0s3r]

Hmmm...that was a nice explanation.And I think you're right.
Thanks!