C++ Copy Constructor and Assignment Operator Define

C++ Copy Constructor and Assignment Operator Define

Could anybody help me correct the following copy constructor and assignment operator?

1. as you see, assignment operator seems to work well; I ran it and it works.
Do I define the assignment operator correct? Please let me know.

2. This crashes with the copy constructor...
How can I fix the copy constructor?

 ``123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102`` `````` #include using namespace std; class IntP { private: unsigned int* counts; unsigned int numP; unsigned int size; public: IntP(); // Default Constructor IntP(int n); // Constructor IntP(const IntP& a); // Copy Constructor IntP& operator= (const IntP& a); // Assignment Operator ~IntP(); // Destructor void printIntP() const; }; IntP::IntP() // Default Constructor { counts = new unsigned int[101] (); // initialize array of size 101 to all 0s numP = 0; size = 101; } IntP::IntP(int n) // Constructor { counts = new unsigned int[n+1] (); // initialize array of size n+1 to all 0s counts[n] = 1; numP = 1; size = n+1; } // ???????????? // IntP::IntP(const IntP& a) // Copy Constructor { this->size = a.size; this->numP = a.numP; for (int i=0; i < (int) this->size; i++) this->counts[i] = a.counts[i]; } // ??????????? // Correct Implementation? // without delete operator, we have memory leak? but it compiles without error??? IntP& IntP::operator= (const IntP& a) // Assignment Operator { if (this != &a) { delete [] counts; // Get rid of old counts size = a.size; numP = a.numP; counts = new unsigned int[size+1]; counts[size] = 1; for (int i=0; i < (int) this->size; i++) counts[i] = a.counts[i]; } return *this; } IntP::~IntP() { delete [] counts; } void IntP::printIntP() const { cout << "The size of array is " << this->size << endl; cout << "The numP variable becomes " << this->numP << endl; int i = 0; while ( i != (int) this->size ) { cout << counts[i]; if ( i != (int) this->size-1 ) cout << " , "; i++; } cout << endl << endl; } int main (void) { IntP ip2(200); IntP ip3; ip3 = ip2; IntP ip1(100); cout << "Print out ip1 object after IntP ip1(100); " << endl; ip1.printIntP(); IntP ip4(ip1); cout << "Print out ip4 object after IntP ip4(ip1); " << endl; ip4.printIntP(); system("pause"); return 0; }``````
 ``12345678910111213141516`` `````` // Correct Implementation? // without delete operator, we have memory leak? but it compiles without error??? IntP& IntP::operator= (const IntP& a) // Assignment Operator { if (this != &a) { delete [] counts; // Get rid of old counts size = a.size; numP = a.numP; counts = new unsigned int[size+1]; counts[size] = 1; for (unsigned int i=0; i < this->size; i++) // CHANGED counts[i] = a.counts[i]; } return *this; }``````

Looks good to me. I changed the for() loop to use unsigned int.

1) prefer using std::size_t from the cstddef header for sizes, instead of int:
 ``123456`` ``````#include // ... for (std::size_t i = /* ... */ ) // ... ``````

2) learn and use the function templates from the algorithm header instead of reinventing the wheel:
 ``12345678`` `````` for (unsigned int i=0; i < this->size; i++) // CHANGED counts[i] = a.counts[i]; // same as... #include std::copy(a.counts, a.counts + size, counts);``````

http://cplusplus.com/reference/algorithm/

 without delete operator, we have memory leak? but it compiles without error???

A memory leak is not an error detected by the compiler.
Also, there is a delete[] in your destructor so everything seems to be fine.

` IntP::~IntP() { delete [] counts; }`

 ``12345678910`` `````` IntP::IntP(const IntP& a) // Copy Constructor { this->size = a.size; this->numP = a.numP; this->counts = new unsigned int[size]; // ADDED for (int i=0; i < (int) this->size; i++) this->counts[i] = a.counts[i]; }``````
Last edited on
 ``123456789`` `````` // ???????????? // IntP::IntP(const IntP& a) // Copy Constructor { this->size = a.size; this->numP = a.numP; for (int i=0; i < (int) this->size; i++) this->counts[i] = a.counts[i]; }``````

This is wrong. count points to some random place in memory when you dereference it. The use of this-> is not necessary here.

 ``123456`` `````` IntP::IntP(const IntP& a) : counts(new unsigned[a.size]), numP(a.numP), size(a.size) { for (unsigned i=0; i < size; ++i) counts[i] = a.counts[i]; }``````

What is numP supposed to represent?
Topic archived. No new replies allowed.