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
– Konstantin, Commented Jan 5, 2014 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
– Konstantin, Commented Jan 5, 2014 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
– Etherealone, Commented Jan 5, 2014 at 18:32

Community · Accepted Answer · 2017-05-23 10:25:30Z

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, 2017 at 10:25

CommunityBot

11 silver badge

answered Jan 5, 2014 at 18:36

Abhijit

64k20 gold badges143 silver badges209 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

egur Over a year ago

You mean missing an operator=. A copy ctor is not used here.

Abhijit Over a year ago

@egur: Good Point. Overlooked on the usage :-)

Konstantin Over a year ago

Ok, thanks a lot! I'm not sure if I'm certain about the implementation but at least it narrows down the search.

egur Over a year ago

Look in my answer for details

egur · Accepted Answer · 2014-01-05 18:48:00Z

0

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;        
    }

edited Jan 5, 2014 at 18:48

answered Jan 5, 2014 at 18:42

egur

8,0102 gold badges30 silver badges47 bronze badges

Collectives™ on Stack Overflow

C++ destructing array of pointers

2 Answers 2

4 Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related