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
– Slava, Commented Apr 7, 2016 at 16:49
@Slava is in't it connected? I don't think it is another question. — sebap123
– sebap123, Commented Apr 7, 2016 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
– Slava, Commented Apr 7, 2016 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
– sebap123, Commented Apr 7, 2016 at 16:54

Slava · Accepted Answer · 2016-04-07 16:32:28Z

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 7, 2016 at 16:32

answered Apr 7, 2016 at 16:24

Slava

44.4k2 gold badges54 silver badges100 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

sebap123 Over a year ago

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?

Slava Over a year ago

I suggest you do not delete them, just change them accordingly to logic (that erased element is not pointed to anymore)

sebap123 Over a year ago

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?

Slava Over a year ago

@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.

sebap123 Over a year ago

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.

Collectives™ on Stack Overflow

Deleting pointer to object in unordered_map

1 Answer 1

5 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related