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?
Please help me.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
|
#include <iostream>
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;
}
|
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
|
// 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.
Advices:
1) prefer using
std::size_t from the
cstddef header for sizes, instead of
int:
1
2
3
4
5
6
|
#include <cstddef>
// ...
for (std::size_t i = /* ... */ )
// ...
|
learn and use the function templates from the
algorithm header instead of reinventing the wheel:
1
2
3
4
5
6
7
8
|
for (unsigned int i=0; i < this->size; i++) // CHANGED
counts[i] = a.counts[i];
// same as...
#include <algorithm>
std::copy(a.counts, a.counts + size, counts);
|
| 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; }
Edit: in your copy constructor...
1
2
3
4
5
6
7
8
9
10
|
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
1
2
3
4
5
6
7
8
9
|
// ????????????
//
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.
1
2
3
4
5
6
|
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.