C++ destructing array of pointers

Question

So here's a C++ exercise on dynamic memory allocation and objects creation. Basically - a custom class Student and a custom class Group which keeps an array of pointers to Students inside. There's obviously a problem in Group's destructor but I've spent hours reading manuals and surfing forums and still cannot understand what I'm doing wrong.

Any comments are welcome.
UPD: the problem is - error on exit. "Debug assertion failed... _BLOCK_TYPE_IS_VALID..."

class Student{
    char *firstName;
    char *lastName;

public:

    Student(): firstName(NULL), lastName(NULL){}

    Student(const char *fname, const char *lname){
        firstName = new char[32];
        lastName = new char[32];
        strcpy_s(firstName, 32, fname);
        strcpy_s(lastName, 32, lname);
    }

    void Show(){
        cout << firstName << " " << lastName << endl;
    }

    ~Student(){

        if(lastName != NULL)
            delete[] lastName;

        if(firstName != NULL)
            delete[] firstName;
    }
};

class Group{
    Student *students;
    int studentCounter;
public:

    Group(){
        students = NULL;
    }

    Group(const int size){
        students = new Student[size];
        studentCounter = 0;
    }

    void Push(Student *student){
        students[studentCounter] = *student;
        studentCounter++;
    }

    void ShowAll(){
        for(int i = 0; i < studentCounter; i++){
            students[i].Show();
        }
    }

    ~Group(){

        if(students != NULL)
            delete[] students;                //problem here?
    }
};

void main(){

    Student jane("Jane", "Doe");
    Student john("John", "Smith");
    Group group(2);

    group.Push(&jane);
    group.Push(&john);

    group.ShowAll();

    _getch();
}

Oh, indeed )) Some error on program exit. _BLOCK_TYPE_IS_VALID — Konstantin, Jan 05 '14 at 18:29
Yes, I've read about rule of three but still don't get what's wrong. Part about copying object? What should it look like? — Konstantin, Jan 05 '14 at 18:30
Offtopic - No need to check ´!= NULL´ before ´delete´. Deleting null is safe (checking is usually micro tiny bit faster though because jump to delete function is avoided). — Etherealone, Jan 05 '14 at 18:32
Your title is misleading, you don't have an array of pointers. — Mat, Jan 05 '14 at 18:34
Isn't Push() adds hew Student pointer to students array? Did I get this part wrong too? — Konstantin, Jan 05 '14 at 18:36
[OT]: btw, you may use `std::string` and `std::vector`, or at least `char firstName[32]` — Jarod42, Jan 05 '14 at 19:14

score 5 · Accepted Answer · edited May 23 '17 at 10:25

5

Your Student Class is missing a copy assignment Operator, in absence of which, the default assignment operator provided only does a shallow copy.

When you create the student object and push in onto Group, both the student object and the one Group holds in its array has the same reference to the firstName and lastName array in absence of an assignment operator, which was supposed to create a clone of these data structures.

So when the Student Objects were deleted during stack unwinding, it results in a double delete as the arrays were already deleted when Group was destroyed.

edited May 23 '17 at 10:25

Community

1
1

answered Jan 05 '14 at 18:36

Abhijit

62,056
18
131
204

1

You mean missing an `operator=`. A copy ctor is not used here. – egur Jan 05 '14 at 18:39
@egur: Good Point. Overlooked on the usage :-) – Abhijit Jan 05 '14 at 18:41
Ok, thanks a lot! I'm not sure if I'm certain about the implementation but at least it narrows down the search. – Konstantin Jan 05 '14 at 18:45
Look in my answer for details – egur Jan 05 '14 at 18:48

egur · Answer 2 · 2014-01-05T18:48:00.450

The problem is:

students[studentCounter] = *student;

You need to write an assignment operator (operator=) so the internal buffers are copied and not only addresses are copied.

What happens is that the firstName, lastName vars are deleted twice.

class Student{
//...
public:

    Student& operator=(const Student& rhs) {
        delete firstName;
        delete LastName;
        firstName = new char[32];
        lastName = new char[32];
        strcpy_s(firstName, 32, rhs.firstName);
        strcpy_s(lastName, 32, rhs.lastName);
        return *this;        
    }

C++ destructing array of pointers

2 Answers2