Destuctors and allocated memory

[AbR]

main.cpp

#include <iostream>
#include <vector>
#include "class_dish.h"
#include "class_dish_data.h"

int main() {

    std::vector<Dish> Common;
    createCommon(Common);
    
    // code below will not execute
	
    for(int i=0; i<2; i++) {
        std::cout<<"\n"<<Common.at(i).foodName;
    }
    return 0;
}

class_dish.h

#ifndef CLASS_DISH_H_INCLUDED
#define CLASS_DISH_H_INCLUDED
#include <string>

class Dish {
    public:
        Dish(int, std::string, std::string*);
        ~Dish();
        int nStk;
        std::string foodName;
        std::string* indkoeb;
};

#endif // CLASS_DISH_H_INCLUDED 

class_dish.cpp

#include "class_dish.h"

Dish::Dish(int noIndkoeb, std::string name, std::string* buyThis) {
    nStk = noIndkoeb;
    foodName = name;
    indkoeb = new std::string[nStk];

    for(int i=0; i<nStk; i++) {
        indkoeb[i] = buyThis[i];
    }
};

Dish::~Dish() {
    delete[] indkoeb;  //if I skip this, program runs fine.
};

class_dish_data.h

#ifndef CLASS_DISH_DATA_H_INCLUDED
#define CLASS_DISH_DATA_H_INCLUDED

void createCommon(std::vector<Dish>&);

#endif // CLASS_DISH_DATA_H_INCLUDED 

class_dish_data.cpp

#include <vector>
#include "class_dish.h"

std::string com01[] = { "text_a01", "text_a02", "text_a03", "text_a04" };
std::string com02[] = { "text_b01", "text_b02", "text_b03", "text_b04", "text_b05", "text_b06", "text_b07", "text_b08" };

void createCommon(std::vector<Dish> &Common) {
    Dish Com01(4, "name01", com01);
    Dish Com02(8, "name02", com02);

    Common.push_back(Com01);
    Common.push_back(Com02);
}   // <- fails here, when destructor is called 

If I change the destructor to this:

Dish::~Dish() {
    std::cout<<"\nobject destroyed";
};

The program outputs this:

object destroyed
object destroyed
object destroyed
name01
name02
object destroyed
object destroyed

The destructor is called 3 times (from only 2 objects!), but I can access the objects in the vector and display the object names, before the destuctor is called 2 more times when the vector goes out of scope (end of program).

My real question is, how do I ensure that the memory allocated by the constructor(s) is properly released?

[MikeyBoy]

In createCommon, you're creating two local objects, Com01 and Com02.

Then, when you push them onto your vector, a copy of each of those is created on the vector.

Since you haven't defined a copy constructor for Dish, those copies are shallow copies. This means that indkoeb in Com01 is pointing to the same memory as the copy of it in the first element of the vector.

When createCommon exits, Com01 falls out of scope and is destroyed, and the memory pointed to by Com01.indkoeb is freed by the destructor. This means that that the memory pointed to by Common[0].indkoeb is now pointing at memory which has been freed. This is what causes your crash.

If you want to create copies of your Dish objects, you need to write a copy constructor for the class, that allocates new memory for indkoeb in the copy.

[keskiverto]

"If you implement one, you usually have to implement all three."


Copy constructor. It is called, when you push_back. You did not implement it, so the compiler did, but that default implementation does not allocate memory dynamically. It just copies values.

After first push_back in createCommon() both Com01.indkoeb and Common.at(0).indkoeb point to same memory. On return from createCommon() the Com01 is destroyed. After that the Common.at(0).indkoeb still points to memory that has already been released.

[Cubbi]

..or you could make indkoeb a vector of strings and not worry about destructor/copy constructor/copy assignment. Let the compiler do its job.

[MikeyBoy]

Yeah - what Cubbi said. Learn to use STL vectors as soon as possible, instead of using C-style arrays. You'll be glad you did.

[AbR]

Thanks for all the answers, explanations and suggestions; it works perfectly using vector<string> instead of the array.

Just out of curiosity, can anyone explain why the destructor was called 3 times in the initial code (upon returning from createCommon)?

[keskiverto]

http://stackoverflow.com/questions/3167272/how-does-c-stdvector-work

Vector has capacity. You start with empty vector, so possibly:

// size==0, capacity==0

push_back(A)
  allocate space1 for one
  copy A to space1
// size==1, capacity==1

push_back(B)
  allocate space2 for two
  copy one value from space1 to space2
  copy B to space2
  deallocate space1 ** This is where the first destructor would fire
// size==2, capacity==2

return from function
  two local variables self-destruct

return from main()
  vector deallocates space2 and destroys the two objects in it

You could verify that by adding debug prints here and there, testing effect of vector::reserve() and different number of push_back()s.

[AbR]

Thank you