Click here to Skip to main content
15,913,854 members
Please Sign up or sign in to vote.
0.00/5 (No votes)
See more:
Hi all,
im using a program in which i need to use a STL map in a struct as shown in the program below:

MIDL
typedef struct sNode
        {
          char * p;
          map<string,int> myMap;         
        }tsNode;


int main()
{
tsNode * pNode;
pNode = (tsNode *)malloc(sizeof(tsNode));
pNode -> p = (char *)malloc(sizeof(char)* 10);

strcpy(pNode -> p,"prasad");
pNode -> myMap.insert(make_pair("A",1)); //Program crashes here:confused:
pNode -> myMap.insert(make_pair("B",2));

for(map<string,int>::iterator it = pNode -> myMap.begin();it!= pNode -> myMap.end();++it)
cout<<it->first<<" : "<<it->second<<"\n";
cout<<"pNode->p"<<" : "<<pNode->p;
cout<<"\n";
return 0;
}

Can anyone tell what is wrong in the above code.
Any suggestions or ideas are greatly welcomed :)
Posted

The first thing that's wrong with the code is using C functions all over the place. The second thing is the map constructor's not running, because you're using C functions (specifically malloc) all over the place. Thirdly even if it did run it'd leak resources (memory) as you're not using the C functions you are using properly (where's the two calls to free?).

So... Don't use malloc. Don't use strcpy, use a string instead. There's no point using the typedef for a struct - in C++ you don't need them in half the places you do in C. Doing that lot'll make your code far simpler:

typedef std::map<std::string, int> value_map;

struct node
{
    std::string name_;
    value_map values_;
};

std::ostream &operator<<( std::ostream &str, const value_map::value_type &v )
{
    return str << v.second;
}

int main()
{
    std::tr1::shared_ptr<node> n( new node );
    n->name_ = "prasad";
    n->values_[ "A" ] = 1;
    n->values_[ "B" ] = 2;

    for( value_map::const_iterator iter = n->values_.begin();
         iter != n->values_.end(); ++iter )
    {
	std::cout << *iter << std::endl;
    }
}


It could just be me but note how much easier main is to follow... And it won't run any slower.

I'm also a bit worried about anyone that calls a class node, this seems to imply that they're about to go forth and spew out a tree or list class of their own. Don't do it unless std::list and std::map don't work for your requirements!

Good luck,

Ash
 
Share this answer
 
v3
Comments
Prasad A 27-Sep-10 6:38am    
Thank you for your helpful suggestions:)
Nope, but you must change
Prasad A wrote:
pNode = (tsNode *)malloc(sizeof(tsNode));

into
pNode = new tsNode();

because malloc doesn't call constructors.

BTW: As matter of style, doesn't use C allocation functions (like malloc, calloc, etc...) when developing C++ programs.




:)
 
Share this answer
 
Comments
Prasad A 27-Sep-10 6:30am    
Thank you... it was helpful :)
i have one more query concerned with the above program.
Before freeing the pNode, it is required to free pNode -> p and pNode -> myMap
delete pNode -> p would work fine but how do i clear the myMap contents.
Does the pNode -> myMap.clear() do the work for me or do i need to free individual elements from map using pNode -> myMap.erase() function.
I wonder how did you code that much when you don't have basic knowledge about pointers, memory allocation routines, string function etc? :sigh:
 
Share this answer
 
Comments
discret 4-Jan-12 0:04am    
I wonder how u commenting like this without knowing/giving answer
Ajay Vijayvargiya 4-Jan-12 1:20am    
Becauase the OP doesn't know only 'new' will call the constructor, and 'malloc' won't call it. And therefore `std::map` doesnt get initialized, and hence the crash.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900