C++ map::erase() does not erase data

Question

I'm trying to test C++ map::erase() with the following code:

//file user.h
#include <string>
#include <fstream>
#include <cstring>

using namespace std;

class User {
    string name;
    int id;
public:
    User(const string& name, int id) : name(name), id(id) {}
    int getID() const {return id;}
    ~User(){}
};

//file main.cpp
#include "user.h"
using namespace std;

typedef map<string, User*> Dict;

int main()
{
    Dict dict;
    dict["Smith"] = new User("Smith", 666); //Id = 666
    dict["Adams"] = new User("Adams", 314); //Id = 314


    auto it = dict.find("Adams"); //look for user 'Adams'

    if (it == dict.end())         

    //show 'not Found' if didn't find 'Adams'
    cout << "not Found" << endl; 

    else
    //else, show the Id = 314
    cout << "id1: " << it->second->getID() << endl;


    //Here I think there is a problem
    //I ask to delete Adams from the list
    dict.erase(it);
    //So in this print the ID shouldn't be found
    cout << "id2: " << it->second->getID() << endl;

    return 0;
}

After I try to delete the item from the list it seems like it is not deleted as the program shows the following:

pc@pc:~/Test$ ./main
id1: 314
id2: 314

As I understand id2 shouldn't show any value. Is this good or did I misunderstood the use of erase. If yes, how can I delete the item after it is shown?

Well, I'll ask you -- what did you expect that last `cout` line to do when you gave it something that was erased? A blank line of output? A crash? A message box saying "you have an error"? Nothing stops you from writing code that is faulty. The problem is that when you run faulty code in C++, you cannot predict what will happen. — PaulMcKenzie, Feb 26 '16 at 17:12
You are using namespace std, both in your main.cpp file and in your header file. [This is not recommended](http://stackoverflow.com/q/1452721/3982001), especially [in headers it should never be included](http://stackoverflow.com/q/5849457/3982001). — Fabio says Reinstate Monica, Feb 26 '16 at 17:23

pm100 · Answer 1 · 2016-02-26T17:17:02.497

5

you are in undefined behavior land. You are using an iterator (it) after you have modified the map. Anything can happen - including apparently working (a bit). You shoud redo

auto it = dict.find("Adams"); //look for user 'Adams'

this will not find anything

edited Feb 26 '16 at 17:17

answered Feb 26 '16 at 17:09

pm100

48,078
23
82
145

score 3 · Accepted Answer · answered Feb 26 '16 at 17:11

3

Basically you have undefined behavior calling

dict.erase(it);
//So in this print the ID shouldn't be found
cout << "id2: " << it->second->getID() << endl;

The iterator variable isn't somehow reset when it was used with dict.erase(it);.

Also you should take care to call delete before using erase(). Otherwise you would leak memory.

answered Feb 26 '16 at 17:11

πάντα ῥεῖ

1
13
116
190

I dort unterstand the delete part. Should he delete the iterator before calling erase? O_o – Vincent Feb 26 '16 at 17:48
@πάντα ῥεῖ thanks, just one more question: if smart pointers are used then it is not necessary to call `delete`, but in this case how can the iterator `it` be deleted? If I add this line `delete it;` before `erase()` I get this compiling error: `error: type ‘struct std::_Rb_tree_iterator, User*> >’ argument given to ‘delete’, expected pointer delete it;` – Marco Feb 26 '16 at 17:49
@MarcoSurca Yes, using a smart pointer is the way to go to have dynamically allocated object instances automatically be deleted when there's no more reference for these. The correct type declaration should be `std::map> dict;` – πάντα ῥεῖ Feb 26 '16 at 17:53
@πάντα ῥεῖ but is there any way to delete the iterator `it` in this case in which smart pointers are not used? – Marco Feb 26 '16 at 17:54
1

@MarcoSurca I'm not sure what you mean with _delete the iterator `it`_, but assumed you mean to `delete it->second;`, do it before calling `dict.erase(it)`. – πάντα ῥεῖ Feb 26 '16 at 17:58

score -2 · Answer 3 · answered Feb 26 '16 at 17:08

-2

You're erasing a pointer from the map, but the object being pointed to by the map isn't being erased. You need to spend some time learning about memory management in c++.

answered Feb 26 '16 at 17:08

Xirema

19,889
4
32
68

3

And even if it was erased, he accessing invalid iterator which is undefined behaviour. He could and probably would get same result even if he called delete on that object – Revolver_Ocelot Feb 26 '16 at 17:09

C++ map::erase() does not erase data

3 Answers3