CODEWITHSUNDEEP
c++dictionary
srrvnn
srrvnn

Reputation: 610

std::map values lost after returning

How to handle memory properly while passing a map from one function to another.

I have a function that returns a map that it builds. The value object is a Class foo. I print foo at three different locations, and all of them give different values. The first time, it gives the right values. The second and third are garbage.

I know I have to make the Foo object pointers at the right places.

I want to know where?

std::map<int,Foo*> function_that_returns_the_map(){

  std::map<int,Foo*> myMap; 

  {

    int v = 0; 
    Foo *b = new Foo();

    // PRINTING FOO FIRST

    std::cout<<""<<*b<<endl;

    myMap.insert(std::pair<int,Foo*>(v,b))

  }

  // PRINTING FOO AGAIN 

  for(map<int,Foo*>::iterator it = myMap.begin();
  it != myMap.end(); ++it)
  {      
    std::cout << " " << *(it->second) << "\n";
  }


  return myMap;
  }

std::map<int, Foo*> myMap;
myMap = function_that_returns_the_map();

//PRINTING FOO AGAIN.

 std::map<int, Board*>::iterator it = myMap.begin();
 for (it=myMap.begin(); it!=myMap.end(); ++it)
   cout<<" "<<*(it->second)<<endl;

Click here to see my actual code.

Update: The member variables of Foo were not being allocated using the 'new' operator. Hence, they were going out of scope and having garbage values once they went out of scope.

Upvotes: 2

Views: 4907

Answers (3)

JBentley
JBentley

Reputation: 6260

You had quite a few minor mistakes in your code (which I assume to just be typos). I've fixed those and provided a Foo class, and it compiles and runs fine, printing the correct value in all three places:

#include <iostream>
#include <map>

struct Foo
{
   Foo() : someValue(5) {};
   int someValue;
};

std::map<int,Foo*> function_that_returns_the_map()
{
   std::map<int,Foo*> myMap;
   {
      int v = 0; 
      Foo *b = new Foo();
      std::cout << (*b).someValue << std::endl; // PRINTING FOO FIRST
      myMap.insert(std::pair<int,Foo*>(v,b));
   }

   // PRINTING FOO AGAIN
   std::map<int, Foo*>::iterator it = myMap.begin();
   for(it; it != myMap.end(); ++it)
   { 
      std::cout << it->second->someValue << "\n";
   }
   return myMap;
}

int main()
{
   std::map<int, Foo*> myMap;
   myMap = function_that_returns_the_map();

   //PRINTING FOO AGAIN.

   std::map<int, Foo*>::iterator it = myMap.begin();
   for (it; it!=myMap.end(); ++it)
   std::cout << it->second->someValue << std::endl;
   return 0;
}

Click here to view the output.

The issue therefore must be in something you haven't mentioned in your question. To be able to assist further, we'll need to see the real code.

Upvotes: 1

Orion Edwards
Orion Edwards

Reputation: 123612

Where you've commented "PRINTING FOO HERE" how are you even compiling it? You can't print Foo via the b local variable in either of those places because it is out of scope. You instead must retrieve the object out of the map

I made the following changes to your code, and it all works fine:

class Foo
{
public:
    const int Value;
    Foo(int value) : Value(value) // add some kind of identifier to Foo so we can check it's not garbage
    { }
};

std::map<int,Foo*> function_that_returns_the_map()
{
    std::map<int,Foo*> myMap; 

    { // introducing a new scope

        int v = 0; 
        Foo *b = new Foo(98765);

        // PRINTING FOO HERE.
        std::cout << b->Value << std::endl;

        myMap.insert(std::pair<int,Foo*>(v,b));
    } // v, b go out of scope, are no longer accessible

    // PRINTING FOO HERE.
    std::cout << myMap[0]->Value << std::endl; // we can't use v, b anymore, so go fish in the map to find the Foo

    return myMap;
}

void main()
{
    std::map<int, Foo*> myMap;
    myMap = function_that_returns_the_map();

    //PRINTING FOO HERE.
    std::cout << myMap[0]->Value << std::endl; // we can't use v, b anymore, so go fish in the map to find the Foo
}

The key is the scoping. Curly braces in C++ mean scope. Any local variables declared inside that scope can't be used from outside it. Hopefully this will help explain it. If not, please comment about anything that doesn't make sense.

PS: Remember, because you've used new to create your Foo objects, you MUST use delete somewhere to clean them up, otherwise your program will memory leak. Because of this, people usually don't put pointers directly into maps or lists. Instead, just put a copy of the Foo object in (no pointers), or else use a wrapper such as a shared_ptr to manage the deleting for you.

Upvotes: 0

Joseph Mansfield
Joseph Mansfield

Reputation: 110648

Make sure you actually create the std::pair with the right values:

myMap.insert(std::pair<int,Foo*>(v, b));
//                               ^^^^

Or make use of std::make_pair:

myMap.insert(std::make_pair(v, b));

Upvotes: 1

Related Questions