Reputation: 61
The following code is just a raw example reference. Everything works fine, but when the program is about to close it stuck for about a few seconds. It's all because of the wrong way of deleting memory I am assuming. Please find below code snippet:
class Test {
private:
static int count;
static Test* arr;
public:
int num;
Test() {}
Test(int val) : num(val) {
arr[count] = *this;
count++;
}
~Test() {
delete[] arr;
}
void print() {
for (int a = 0; a <= count - 1; a++) {
std::cout << arr[a].num << std::endl;
}
}
};
int Test::count = 0;
Test* Test::arr = new Test[2];
int main(int argc, char** argv) {
Test t1(10);
Test t2(20);
t2.print();
return 0;
}
Upvotes: 5
Views: 95
Reputation: 22152
Don't use raw new
and delete
at all:
class Test {
private:
static int count;
static std::vector<Test> arr;
public:
int num;
Test() {}
Test(int val) : num(val) {
arr[count] = *this;
count++;
}
// No user-defined destructor needed!
void print() {
for (int a = 0; a <= count - 1; a++) {
std::cout << arr[a].num << std::endl;
}
}
};
int Test::count = 0;
std::vector<Test> Test::arr{2};
int main(int argc, char** argv) {
Test t1(10);
Test t2(20);
t2.print();
return 0;
}
(requires #include<vector>
)
This does the same thing, but does not cause you any trouble with finding the correct place to delete
. std::vector
manages that for you.
Note however that whatever the purpose of this class is, it probably should not be limited in the number of Test
it can create:
class Test {
private:
static std::vector<Test> arr;
public:
int num;
Test() {}
Test(int val) : num(val) {
arr.push_back(*this); // Add new element!
}
// No user-defined destructor needed!
void print() {
for (int a = 0; a < arr.size(); a++) {
std::cout << arr[a].num << std::endl;
}
}
};
int Test::count = 0;
std::vector<Test> Test::arr; // Start empty!
int main(int argc, char** argv) {
Test t1(10);
Test t2(20);
t2.print();
return 0;
}
Now you aren't limited to using only two elements and you don't have to keep track of count
either!
If you want to use new
(which you shouldn't), then you need to call delete[]
exactly once for each new[]
.
In your code you call new[]
once to initialize arr
at program startup. You call delete[]
each time a Test
is destroyed and you have four Test
instances in your program: 2 variables of type Test
in main
and 2 objects of type Test
in the array that you allocate for arr
. That means you are deleting the array pointed to by arr
four times! That causes undefined behavior.
To do it correctly, you need to deallocate it once, when it is not needed anymore:
class Test {
private:
static int count;
static Test* arr;
public:
int num;
Test() {}
Test(int val) : num(val) {
arr[count] = *this;
count++;
}
// No user-defined destructor needed!
void print() {
for (int a = 0; a <= count - 1; a++) {
std::cout << arr[a].num << std::endl;
}
}
};
int Test::count = 0;
Test* Test::arr = new Test[2]; // Executed once at startup
int main(int argc, char** argv) {
Test t1(10);
Test t2(20);
t2.print();
delete[] arr; // We don't need the array anymore, so `delete[]` it *once*
return 0;
}
Please also note that I doubt the you are approaching whatever problem you actually have the correct way with the design of this class. If your intention is to register all objects of type Test
that are created in a global registry, then you should be warned that you are saving copies of the Test
objects in arr
.
Discussing other approaches to such a problem is out-of-scope of the question, especially if I don't know the actual intended use. So I will stop here.
Upvotes: 6
Reputation: 6039
You are deleting the static memory in arr
multiple times which leads to undefined behavior.
Upvotes: 0