Deleting pointer to object in unordered_map

Question

I am trying to delete references to unordered_map element. The part of my classes responsible for that looks like this:

class Edge:
{
    public:
        Node* from;
        Node* to;
}

class Node:
{
    public:
        std::string name;
        bool to_delete;
}

class Graph:
{
    public:
        std::unordered_map<std::string, Node> nodes;
        std::vector<Edge> edges;
}

and in my main file code I am doing something like this:

    Node n("My node");
    graph.nodes.insert({n.name,n});
    edge.from = &graph.nodes[n.name];

    // Some other stuff

    for(auto& edge : graph.edges)
    {
        if(edge.from->to_delete)
        {
            graph->nodes->erase(edge.from->name);
            delete edge.from;
            edge.from = NULL;
        }
        if(edge.to->to_delete)
        {
            graph->nodes->erase(edge.to->name);
            delete edge.to;
            edge.to = NULL;
        }

        if(edge->from && edge->to)
            DoSomethingWithNodes(edge->from, edge->to);
        else
            removeEdge(edge);
    }

Currently I am deleting pointers based on this answer but I am getting segmentation error on line with delete. In this same answer there is also suggestion to use smart pointers. I am not sure about using here shared_ptr. I have option here, that multiple edges will have pointers to one node object, but what will actually happen when I'll erase node from graph's unordered_map. Will I get false for last if/else condition if it was pointing there? I don't fully understand that.

EDIT:

Assume that I want to display the name of a node before it gets deleted. So I'll have something like that:

for(auto& edge : graph.edges)
{
    if(edge.from->to_delete)
    {
        printf("Node to delete: %s",edge.from->name.c_str());
        graph->nodes->erase(edge.from->name);
        edge.from = nullptr;
    }
    if(edge.to->to_delete)
    {
        printf("Node to delete: %s",edge.to->name.c_str());
        graph->nodes->erase(edge.to->name);
        edge.to = nullptr;
    }

    if(edge->from && edge->to)
        DoSomethingWithNodes(edge->from, edge->to);
}

Right now, there is no protection against null pointers, because as I've experienced edge.from->to_delete is somehow, sometimes returning true. What I've tried is changing if conditions to:

if(edge.from && edge.from->to_delete)

but it doesn't help at all.

Ask one question at a time. If your program does not behave as expected publish MCVE — Slava, Apr 07 '16 at 16:49
@Slava is in't it connected? I don't think it is another question. — sebap123, Apr 07 '16 at 16:51
I already answered your question about "Deleting pointer to object in unordered_map", why your code does not work as expected is different question. First of all there is not enough information to answer it. — Slava, Apr 07 '16 at 16:53
@Slava Ok, so that's also an answer now. At least I know (or can assume) that deleting of pointer is working. — sebap123, Apr 07 '16 at 16:54

Slava · Accepted Answer · 2016-04-07T16:32:28.717

2

Currently I am deleting pointers based on this answer

You did not understand that answer - you can only delete objects which created by new and you control ownership for them, objects in your case are managed by std::unordered_map as you store them by value, not pointer. So in this case you cannot call delete on those objects and calling std::unordered_map::erase() is enough for object being deleted.

Examples:

std::unordered_map<std::string,Someclass> mymap;
mymap.insert( std::make_pair( "foobar", Someclass() ); // adding object by value
...
mymap.erase( "foobar" ); // object managed by map and will be deleted

or when you need to call delete:

std::unordered_map<std::string,Someclass *> mymap;
mymap.insert( std::make_pair( "foobar", new Someclass() ); // adding object by pointer
...
auto f = mymap.find( "foobar" );
if( f != mymap.end() ) {
    delete f->second; // you need to delete object that you created by new before
    mymap.erase( f );
}

edited Apr 07 '16 at 16:32

answered Apr 07 '16 at 16:24

Slava

43,454
1
47
90

Thank you for information. I wasn't aware that in this case this isn't applicable. I thought that in this case `new` is called implicitly. So what you suggest to delete this pointers? – sebap123 Apr 07 '16 at 16:28
I suggest you do not delete them, just change them accordingly to logic (that erased element is not pointed to anymore) – Slava Apr 07 '16 at 16:34
So `edge.from` for example after `erase()` will point to some trash, right? Shouldn't I do something like `edge.from = nullptr;`? Because how will I know if is safe to read name from this node? – sebap123 Apr 07 '16 at 16:36
@sebap123 yes `edge.from` will become dangling pointer, and yes you should change it, if `nullptrr` is correct value depends on your program logic which is not clear fro me. – Slava Apr 07 '16 at 16:40
`nullptr` is for now correct value I'd say. I've edited my question to better explain where I have error. Maybe this will help. – sebap123 Apr 07 '16 at 16:50

Deleting pointer to object in unordered_map

1 Answers1