How do I make a vector reallocate with out calling a destructor?

Question

In this code:

Texture * a = new Texture();
Texture * b = new Texture();
buttons.push_back(*a);
buttons.push_back(*b);

buttons is a vector defined like this:

std::vector<Texture> buttons;

Texture is a class and this is its prototype:

class Texture
{
public:
    Picture * texData;
    HitBox * texCoords;
    Texture();
    ~Texture();
    void render_tex();
};

The 4th line of the first code block is calling the destructor for Texture a. The problem is that this destructor deletes the values pointed to by texData and texCoords which means that when it is reallocated texData and texCoords point to junk data.

Is there a way to make it so that the vector will not call the destructor when it reallocates?

This seems like an XY problem. It would work out much better to fix your class so it can be used with a vector, else not use it with a vector. — chris, Jun 12 '14 at 18:16
You need to define a `noexcept` move constructor for your `Texture` class. See http://stackoverflow.com/questions/8001823/how-to-enforce-move-semantics-when-a-vector-grows. — toth, Jun 12 '14 at 18:18
Define a copy constructor and assignment operator too. Follow [The Rule of Three](http://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming)). — R Sahu, Jun 12 '14 at 18:20
Why does the destructor delete `textData` and `texCoords`? Why are those pointers if the object owns the data, why not direct data members? — Cheers and hth. - Alf, Jun 12 '14 at 18:35

Praetorian · Answer 1 · 2014-06-12T22:58:28.653

5

Since you're dynamically allocating the data members texData and texCoords you'll need to follow the Rule of Three, and define a copy constructor and copy assignment operator too, or your code will be very easily broken, as you've discovered.

But there's a better way! Do no use raw pointers as data members. Use unique_ptrs to hold them. (It's possible you don't need them to be pointers at all, but I'll give you the benefit of the doubt here). You'll still need to define a copy constructor and copy assignment operator, and manually copy the contents of what the unique_ptrs are holding, if you want the class to be copyable.

And to make it moveable you can explicitly =default both the move contructor and move assignment operators.

Also, your code is leaking memory. You allocate those objects dynamically, and then push_back a copy into the vector

Texture * a = new Texture();
buttons.push_back(*a);

You're dereferencing a and adding a copy of it to the vector. Unless you delete a; before it goes out of scope, you've got a memory leak. Use a vector<unique_ptr<Texture>> instead.

vector<unique_ptr<Texture>> buttons;

buttons.push_back(unique_ptr<Texture>(new Texture()));

With C++14 you can use make_unique to avoid having to new the object yourself

buttons.push_back(make_unique<Texture>());

Additionally, if you know the number of buttons you're going to add in advance you can reserve space to avoid reallocations.

buttons.reserve(num_buttons);

Now, chances are you do not actually need to dynamically allocate the Texture objects at all. All you probably need is

vector<Texture> buttons;
buttons.push_back(Texture());

If possible, I'd also define a move contructor and move assignment operator for Texture so that it can be moved when the vector reallocates, instead of being copied.

edited Jun 12 '14 at 22:58

answered Jun 12 '14 at 18:22

Praetorian

106,671
19
240
328

1

Good point, but your example isn't exception-safe. (We need C++14 to hurry up so we can use `std::make_shared`) – Ben Voigt Jun 12 '14 at 18:23
@BenVoigt Getting there ... hold on :) – Praetorian Jun 12 '14 at 18:24
Thank you. The reason for the memory leak was I was trying to figure out what the problem was so this code is a little different from optimal. Additionally If I knew the number of buttons I could just use an array. I gave the answer check to Steger because his is more concise but thank you for the full answer – 0ctoDragon Jun 12 '14 at 19:23
@0ctoDragon: Technically, Steger did not fully answer the question, though his observations are certainly valid. – Lightness Races in Orbit Jun 12 '14 at 20:26
@BenVoigt `std::make_shared` is already in C++11. C++14 will bring `std::make_unique`. – Chnossos Jun 12 '14 at 22:45
@Chnossos: Right you are. And `make_unique` is what I meant ought to be used. – Ben Voigt Jun 12 '14 at 22:46
@BenVoigt And while we're picking away at your comment :) I don't agree with the exception unsafe part either. With or without `make_unique`, if either `new` or the `Texture()` constructor throws, `buttons` is left unchanged. – Praetorian Jun 12 '14 at 22:48
@Praetorian: Without `make_unique`, if reallocation of the vector fails (and throws) then isn't there a danger of leaking the `new Texture()` ? – Ben Voigt Jun 12 '14 at 22:52
@BenVoigt We're both wrong. The code I had wouldn't even have compiled because the `unique_ptr(T*)` constructor is `explicit`. Had I used `emplace_back` instead of `push_back` with my original example, you would be absolutely correct. – Praetorian Jun 12 '14 at 23:00

score 3 · Answer 2 · answered Jun 12 '14 at 18:19

Use smart pointers instead of raw pointers. The following listing uses std::unique_ptr. You can also use std::shared_ptr, if the object have several owners.

std::vector<std::unique_ptr<Texture>> buttons;

std::unique_ptr<Texture> a{new Texture()};
std::unique_ptr<Texture> b{new Texture()};
buttons.push_back(std::move(a));
buttons.push_back(std::move(b));

score 2 · Accepted Answer · answered Jun 12 '14 at 18:20

There is a bigger problem here. Since you have a destructor, you will also need to properly implement an assignment operator and copy constructor. Then your code will work fine as is. Google c++ and The Rule of Three. As mentioned, smart pointers could eliminate the need for the big three since they will handle copying and deleting for you behind the scenes.

score 0 · Answer 4 · edited Jul 03 '22 at 19:01

Does Texture own the Picture and HitBox? In which case, perhaps it should hold Picture and HitBox by value or unique_ptr. Aim for the rule of zero and don't have a destructor at all. Also, is there a good reason why you allocate Texture objects dynamically? I would suggest something like:

#include <vector>

struct Picture {};
struct HitBox {};

struct Texture {
  Picture texData;
  HitBox  texCoords;
  void renderTex() { }
};

int main() {
  std::vector<Texture> textures(2);
}

How do I make a vector reallocate with out calling a destructor?

4 Answers4