0

In my class I wrote

class Game {
private:
    mtm::Dimensions dimensions;
    std::vector<Character*> board;


public:
    explicit Game(int height, int width):dimensions(height,width), board(height*width){
    }

    ~Game() {}
};

But how should I free my vector, I think it leaks memory if I leave the d'tor empty.

  • 3
    That depends on where the `Character*` comes from. If it's allocated with `new`, then yes, you have to `delete` it. – cigien Jun 24 '20 at 15:56
  • 1
    the vector will be destroyed automatically (because it is a member) and also the vector takes care of destroying its elements, but I suppose the leak is the `Character` objects that the elements point to. Do you really need pointers in the vector? – 463035818_is_not_an_ai Jun 24 '20 at 15:56
  • 2
    If they don't have to be pointers, then you can just use `std::vector`. If they do have to be, you can use `std::vector>` … If you want to stay as is, then in `Game`'s destructor, you have to loop over its contents, and delete each `Character` (assuming `Game` 'owns' them) – ChrisMM Jun 24 '20 at 15:57
  • 4
    Let your `vector` hold smart pointers - `std::unique_ptr` or `std::shared_ptr` rather than raw pointers, then the vector elements will automatically die when the `vector` dies. Or just hold objects in the vector rather than pointers to objects. Then cleanup is also automatic. – Jesper Juhl Jun 24 '20 at 15:58
  • It's basically impossible to answer this question without seeing where the pointers come from, and why you are using pointers at all, instead of objects. – cigien Jun 24 '20 at 16:03
  • 2
    Before you so anything you need to [determine who owns](https://stackoverflow.com/questions/49024982/what-is-ownership-of-resources-or-pointers) the allocations being pointed at by the elements of `board`. – user4581301 Jun 24 '20 at 16:06
  • you already have good answers, but I think none of them mentions that in your current code there is no memory leak. Acutally no `Character` is created and all memory allocated in your code is automatically freed – 463035818_is_not_an_ai Jun 24 '20 at 16:34

4 Answers4

4

If Game is not the owner it must not free the memory. The owner has to clean up.

If Game is the owner it can delete all elements in the destructor

~Game() {
    for (auto &character : board) {
        delete character;
    }
}

The better way is to use smart pointers and remove the destructor

std::vector<std::unique_ptr<Character>> board;

You should try to follow the rule of 0

Thomas Sablik
  • 16,127
  • 7
  • 34
  • 62
1

Assuming that your Game owns the objects, then your Game destructor would need to free up the memory. Presumably, the Characters are allocated with new. The below will delete all the Characters in the board, then the class variables (like the vector) will automatically be freed afterward.

~Game() {
   for ( Character *character : board )
      delete character;
}

Unless this is for an exercise with pointers, it's generally recommended not to use bare-pointers, and instead use a smart pointer, such as unique_ptr or shared_ptr.

std::vector<std::shared_ptr<Character>> board;

In this case, there will be no leak, since the Characters will automatically be freed once no one points to them anymore.

Use of shared_ptr vs unique_ptr is dependent on what you're doing.

ChrisMM
  • 8,448
  • 13
  • 29
  • 48
0

The vector is a member variable. Member variables, like all sub objects are automatically destroyed when the super object is destroyed. The implicitly generated destructor does the right thing for vector.

Note that the vector contains pointers. If those pointers point to dynamically allocated objects, and are the only pointers to those objects, then those objects would have to be deleted, or else they would leak. If something else owns the pointed objects, then there is nothing that needs to be done in this destructor.

An example where the implicit destructor is correct. I pretend that the member is public for this example:

{
    Character c;
    Game g;
    g.board.push_back(&c);
} // no leaks

Avoid owning bare pointers. Prefer to use RAII containers or smart pointers if you need ownership of dynamic objects.

eerorika
  • 232,697
  • 12
  • 197
  • 326
-3

the vector destructor won't leak memory you should write destructors for the types used in the vector. Also I wouldn't recommend a pointer to something which dosen't destruct itself.

I think what you are looking for are smart pointers.

just a guy
  • 137
  • 7