C++ Destructor Usage when deleting an object containing a dynamic array as a data member

Question

I've written two classes: Student and Course. The Course class has a Student *students as a data member in its body. In my top class StudentReviewSystem, I have a Course *courses as a data member, and I have a deleteCourse(const int courseId) method which deletes the course with "courseId and resizes "courses". Here is my code of deleteCourse method:

void StudentReviewSystem::deleteCourse(const int courseId){
int index = 0;
bool found = false;
//temp arr
Course *temp = new Course[courseSize];
for(int i = 0; i < courseSize && !found; i++){
    if(courseId == courses[i].getCourseId()){
        index = i;
        found = true;
    }
}
if(found){


    temp = courses;

    courses = new Course[courseSize-1];
    for(int i = 0; i <= index-1;i++ ){
        courses[i] = temp[i];
    }
    for(int i = index+1; i < courseSize; i++){
        courses[i-1] = temp[i];
    }


    delete[] temp;

    cout << "Course has been deleted!" << endl;
    courseSize--;
}

}

When I try to delete temp, I get a "glibc detected" error with a memory map under it and at the then aborted(core dumped) error. But when I comment out the line delete[] temp; , it works. Can you help me, I'm really new to C++.

Thank you...

P.S: When I comment out the ~Course(){ delete[] students;} destructor, it again works. I think I have a really silly problem, thank you again...

You're leaking memory. `Course *temp = new Course[courseSize];` is *always* abandon and leaked in this code. This isn't Java. And by the looks of it your `Course` class isn't practicing the [`Rule of Three`](http://en.wikipedia.org/wiki/Rule_of_three_(C++_programming)) *at all*. — WhozCraig, Nov 23 '13 at 13:33
Replace all those arrays with `std::vector` or some other standard container, and use smart pointers and standard algorithms - you could reduce that function to about three lines that don't contain raw news or deletes. You're leaking a `Course[courseSize]` there. I suspect you're double-deleting students if they can belong to several courses. — Mat, Nov 23 '13 at 13:35

score 2 · Accepted Answer · answered Nov 23 '13 at 13:35

2

You made the typical mistake: your class has a dual responsibility.

The solution is simple:

one class to handle the technical issue of a dynamic (possibly resizable) array
one class to handle the functional requirements, using the first class to achieve its goal

The first class is already available (std::vector), if necessary (aka, your teacher ask it of you) then you can recode a std::vector; otherwise use it as is. Read more on std::vector.

answered Nov 23 '13 at 13:35

Matthieu M.

287,565
48
449
722

Since the bug is in Course, using vector here won't help much. Using vector in Course too might well be the answer. – john Nov 23 '13 at 13:45

score 1 · Answer 2 · edited May 23 '17 at 12:05

Your error is that you are not following the rule of three in your Course class.

See here for full details.

You should follow the rule of three in every class you write. Because you haven't followed the rule of three then any attempt to copy a Course object (like you are trying to do above) will crash your program.

I would say understanding the rule of three is the first step to becoming a decent C++ programmer.

score 0 · Answer 3 · answered Nov 23 '13 at 13:34

0

Maybe you try to delete an array that has not been allocated by new.

answered Nov 23 '13 at 13:34

jdc

319
2
5

C++ Destructor Usage when deleting an object containing a dynamic array as a data member

3 Answers3