CODEWITHSUNDEEP
c++c++11
waas1919
waas1919

Reputation: 2595

Object in map corrupted after retrieving from the map

So I've completely edited my question. I have a map called mTextMap which contains:

typedef std::map<const std::string, Text*>      TextMap;
TextMap mTextMap;

And I have the following methods:

void Foo::setUpGame()
{
    Text text(1,2,3,4); //create a Text object
    mTextMap["MainText"] = &text; //save it in the map!
}

Text& Foo::getText(const std::string name)
{
    auto i= mTextMap.find(name);
    return *(i->second); //Gets a reference to the Text that is inside the map
}

Now if I use this way:

Foo foo;
foo.setUpGame();

Text& myText = foo.getText("MainText"); // Why is this corrupted?

The object myText is completely corrupted!!

Why is this happening?

Upvotes: 0

Views: 1304

Answers (3)

MikeMB
MikeMB

Reputation: 21136

The general problem seems to be, that you think that this line:

mTextMap["MainText"] = &text;

stores the text object in the map. IT DOESN'T! It stores a pointer to the object in the map and the text object itself will - as you said yourself - automatically be destructed at the end of the function. So now your pointer points to a non-existing object, which leads to the observed errors.

There are various solutions to your problem, depending on what exactly, you try to achieve and what you are going to do with your class.

One possibility is to use a map of Text objects (instead of pointers):

typedef std::map<const std::string, Text>      TextMap;
void Foo::setUpGame()
{
    Text text(1, 2, 3, 4); //create a Text object
    mTextMap["MainText"] = text; //copy it into the map!
}

or

void Foo::setUpGame()
{       
    mTextMap.emplace("MainText", Text(1, 2, 3, 4)); //Doesn't require Text to be default constructable
}

Another possibility is to create the text objects on the heap and use smart pointers (e.g. unique_ptr)

typedef std::map<const std::string, std::unique_ptr<Text>>      TextMap;
void Foo::setUpGame()
{
    mTextMap["MainText"] = std::make_unique<Text>(1,2,3,4); //create object on the heap an store a pointer to it in the map
}

The std::unique_ptr will automatically destroy the text object, as soon as the map gets destroyed.

If you really need to have a map of raw pointers for some reason, you can use "new" as explained by David, but don't forget to delete them when you don't use them anymore - c++ doesn't have a garbage collector (like e.g. java) that would take care of this automatically.

Upvotes: 2

David Gourde
David Gourde

Reputation: 3914

When you dynamically allocate memory for your object, it will live as long as you do not explicitly delete it from memory, it is not deleted after you exit the method it was created in, so you can put a pointer to it in a map and it will always be there (just be sure you delete the memory when removing the object from the map).

You can test this with the following simple code, where I declare a new Int in a function, return a pointer to the memory and print it in the other function that received the map (with the pointer in it). It prints correctly, which means the memory was not freed even when out of scope.

#include <iostream>
#include <map>

std::map<std::string, int*> myMap(){
    int* test = new int(1);

    std::map<std::string, int*> ObjMap;
    ObjMap["object"] = test;

    return ObjMap;
}

int main(int argc, const char * argv[]) {
    // insert code here...
    std::map<std::string, int*> mmap = myMap();

    std::cout << *mmap["object"] << std::endl;
    return 0;
}

So to answer your question, create your object dynamically like this: 

Obj* obj = new obj(1,2,3,4);

And it will not be deleted when out of scope. Still, you need to delete the memory yourself unless you use Smart Pointer, like this: delete obj; (when you remove it from the map, to free the memory as it will not be freed automatically).

PS: You should read on how the Stack and Heap works and how Dynamic and Static allocation works (using the stack OR the heap). See this c++ dynamic memory allocation tutorial to have more informations.

Like MikeMB said, using Smart Pointers is easier as you will be sure you deleted the memory, and you will also be sure you never access a deleted memory. See this Stack Overflow topic for smart pointers informations: What is a smart pointer and when should I use one?

Upvotes: 1

Kaitain
Kaitain

Reputation: 960

The "text" object is going out of scope as soon as setUpGame completes. At this point, the heap memory is freed up to be overwritten by any new use of the heap. It is essentially a temporary scratchpad of items that only exists within the scope of a function (or within explicit scope operators inside a function).

David G's advice is sound: read more about the difference between stack and heap memory, and also consider the advice to use smart pointers. However, if you want a cheap, dirty fix to your immediate problem, you can try this:

void Foo::setUpGame()
{
    static Text text(1,2,3,4); // Note use of "static" keyword
    mTextMap["MainText"] = &text; //save it in the map!
}

Whilst I do not advocate the use of static as a shortcut to solving more fundamental architectural memory issues, you can use this as a short-term measure if you're desperate to get things working. Labeling the object as static ensures its lifetime will outlive the scope of the function. But I would not recommend it as a long-term solution to this kind of issue.

Upvotes: 1

Related Questions