Should I delete variables if created them using "new" operator inside map::insert()?

Question

I am writing scene manager which, among other things, contains a map of all objects, or rather pointers to them. I remember to delete my variables created using "new" and i tried doing it in my Scene deconstructor, but I got 0xC0000005 exit code and afaik it means that i tried to use memory that doesnt really exist anymore. Here is my Scene class:

class Scene{
public:
    Scene();
    ~Scene();

    // other functions that I cut for question purposes

    std::map<GLuint, Object*> view_layer;
    Object AddObject(const std::string &title);
};

And I am creating new object in scene like this ("new Object" returns only pointer to newly created variable):

Object Scene::AddObject(const std::string &title) {
    
    // other stuff again

    view_layer.insert(std::make_pair(currentObjectID, new Object));
    return *view_layer.find(currentObjectID)->second;
}

And i tried to delete it like this:

Scene::~Scene() {
    for (auto const& [key, val] : view_layer)
    {
        if(val){
            std::cout << "Deleting object...\n";
            delete [] val;
        }
    }
};

But program crashes after this std::cout, right on delete [] val.

Val should be a reference to the pointer of my object, and delete [] needs pointer, so i guess that i have no syntax issue here.

My question is - Am I doing something wrong or I am just not supposed to delete this because somehow it deletes itself ? Thanks for any kind of help!

`delete` and `delete[]` are two different things. The method used to destruct a resource has to match the one used for creating it. [delete vs delete\[\] operators in C++](https://stackoverflow.com/questions/2425728/delete-vs-delete-operators-in-c) — t.niese, Oct 14 '21 at 18:25
Note that `AddObject` is making and returning _copies_ of the objects in the map. If you apparently don't mind that, then it's simpler not to bother with pointers in the first place and just store `Object`s in the map directly. — Thomas, Oct 14 '21 at 18:26
@Thomas is right: it makes no sense to use pointers and `new` here to begin with. — And the insertion code is inefficient to boot (unnecessary map search). — Konrad Rudolph, Oct 14 '21 at 18:27
@Thomas but how exactly should I store objects in map ? Considering that i must create them during runtime, because amount of them depends on user. Users can create new objects. — Morph, Oct 14 '21 at 18:36
@Morph Same way you create other objects. `view_layer.insert(std::make_pair(currentObjectID, Object()));` … of course you’ll also need to change the type of the map. — Konrad Rudolph, Oct 14 '21 at 18:37
@KonradRudolph ok, I just read that storing objects in map is considered a bad practice, and it is better to only store pointers to them, but there was no explanation to it — Morph, Oct 14 '21 at 18:39
@Morph Where did you read this?! It’s **completely** wrong. — Konrad Rudolph, Oct 14 '21 at 18:41
@KonradRudolph https://stackoverflow.com/a/2281473/16767953 here. But I didn't really read the comments. My bad I only considered the answer. — Morph, Oct 14 '21 at 18:42
In general, C++ Library Containers are better off containing objects than pointers to objects. If they directly contain the objects all of the memory management is handled for you. They get created and deleted on schedule and you don't have to worry about dangling pointers. What you do have to worry about is the stored object must correctly observe [the Rule of Three/Five/Zero](https://en.cppreference.com/w/cpp/language/rule_of_three) because they will be copied around. Mind you, classes should always comply with Three/Five/Zero otherwise you're in for a rough time. — user4581301, Oct 14 '21 at 19:22

score 0 · Accepted Answer · answered Oct 14 '21 at 18:29

0

So.. As t.niese answered 0xC0000005 exit status in my code came from a difference between creating and deleting methods used.

When using new, you should delete it using delete When using new[], you should use delete[]

answered Oct 14 '21 at 18:29

Morph

165
2
9

Should I delete variables if created them using "new" operator inside map::insert()?

1 Answers1