0

I have been trying to get back into C++ but have come across a problem that I have not managed to solve. In the example below I have created a struct Node and a class C. The class contains a map. The class has a get and a set function. The problem I face is that once I execute the get function, the value of the node changes. Below the output of the code below is:

4
32766
32766

As you can see, the value changes once I execute the set function. If anybody knows what the problem is please let me know.

Thanks.

#include <iostream>
#include <map>

using namespace std;

struct Node {
    int value;
    Node(int v):value(v){};
};

class C {
    public:
        map<int, Node*> m;
        C() {};

        void set(int key, int value) {
            Node n = Node(value);
            m.insert(pair<int,Node*>(key, &n));
        }       

        int get(int key) {
            return m[key]->value;
        }
};

int main() {
    C t = C();

    t.set(1,4);
    cout << t.m[1]->value << endl;
    cout << t.get(1) << endl;
    cout << t.m[1]->value << endl;
}
DanteNiewenhuis
  • 1
  • 1
    Your code has undefined behavior. You are storing a pointer to an object in `set` that does not live beyond the call to the function. `m.insert(pair(key, &n));` is not good. – R Sahu Mar 31 '21 at 16:24
  • You probably just want to use `std::map` here and not use pointers. – François Andrieux Mar 31 '21 at 16:51

1 Answers1

0

I think the problem is in your set implementation. Replace it with the following code and it should work as expected.

    void set(int key, int value) {
        Node *n = new Node (value);
        m.insert(make_pair(key, n)); // or m.insert({key, n});
    }
Ashkanxy
  • 2,380
  • 2
  • 6
  • 17
  • This introduces a lot of new problems. You now need to provide or delete at least the copy constructor, the copy assignment operator and provide a destructor. Edit : See [What is The Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) – François Andrieux Mar 31 '21 at 16:35
  • @François Andrieux The suggestion was to resolve the asked question. What you are pointing out is another topic and it is definitely a good practice to release all allocated memories through a destructor. What may need to be done is to iterate through the map and release the second element of each. – Ashkanxy Mar 31 '21 at 17:41