   C++ Map container troubles
  Well, I'm trying to use a map container so that I can look up game items by their name.  We're loading item attributes from an xml reader and creating objects, then storing them in the map for later retrieval.  My problem is that while I'm trying to populate the map with <char *, CGameItem *> key/value pairs, it doesn't insert the newly created items into the map, it just replaces items already in there, even though their keys are entirely different (effectively the map always has a size of 1, the most recently loaded item).  I've tried a few variations of using the map with the same results, so I think my problem is a fundamental misunderstanding about how to use the map container.  Anyway, here's the relevant code snippets.  Any help would be appreciated  =]#ifndef _VDC_CGAMEITEMMANAGER_H_#define _VDC_CGAMEITEMMANAGER_H_#include <string.h> // c string library#include <map>  // Associative array containing references to all items loaded in memory.#include "irrXML.h" // XML reader#include "vdc_CGameItem.h"#include "vdc_SEngineVars.h"using namespace std;using namespace irr::io;class CGameItemManager{public:CGameItemManager( SEngineVars* );~CGameItemManager();CGameItem *getItem(char *name);void load();private:struct strCmp {bool operator()( const char* s1, const char* s2 ) const {return strcmp( s1, s2 ) < 0;}};std::map<char *, CGameItem *, strCmp> gameItems;IrrXMLReader *Pxml;SEngineVars *Pev;};#endifvdc_CGameItemManager.cpp#include "vdc_CGameItemManager.h"CGameItemManager::CGameItemManager( SEngineVars *ev ): Pev(ev){ }CGameItemManager::~CGameItemManager(){ delete Pxml; }CGameItem *CGameItemManager::getItem(char *name){/*CGameItem* item = gameItems[name];printf("trying to load item: %s", item->itemName);if ( item ){// Shallow copy, ok since we dont have dynamic data in game items.CGameItem* foundItem = new CGameItem(*item);printf( "XXXXXXXXX FOUND ITEM, name: %s", foundItem->itemName );return foundItem;}printf("XXXXXXXXX DIDNT FIND ITEM, map size is %d\n", gameItems.size());return NULL;*/std::map<char *, CGameItem *, strCmp>::iterator miter = gameItems.find(name);if (miter != gameItems.end() ){// Shallow copy, ok since we dont have dynamic data in game items.CGameItem* rItem = new CGameItem( *((*miter).second) );printf("********FOUND ITEM, name: %s\n", rItem->itemName);return rItem;}printf("XXXXXXXXX DIDNT FIND ITEM");return NULL;}void CGameItemManager::load(){Pxml = createIrrXMLReader("items.xml");if (!Pxml)return;// Load all the items into the array.// Read file to find the typewhile (Pxml && Pxml->read()){char *item = (char *)Pxml->getAttributeValue("itemName");gameItems[item] = new CGameItem();gameItems[item]->itemName = (char *)Pxml->getAttributeValue("itemName");// Give the item a bunch of attributes.  This part works fine.   }//end while}
  Using char * as a key doesn't do what you want. Switch to std::string instead.   ;Quote:Original post by SiCraneUsing char * as a key doesn't do what you want. Switch to std::string instead.Ahh thanks a ton.  I was under the impression that if I supplied the map with way to compare character pointers that it would work ok.  Do you know why it doesn't work like that?  I'm just curious.   ;  Although it would probably be wise to use string here, you should actually be able to use char*'s as in your posted example. It may be that something is going wrong elsewhere, but I can't really be specific (mostly because I'm not familiar with the irrXML interface).A few unrelated suggestions:1. Don't use leading underscores in your header guards (symbols starting with an underscore followed by a capital letter are reserved for the implementation).2. Using typedef's where appropriate will make your code more clear and readable (as well as more flexible/generic).3. Don't put using directives or declarations in header files (they pollute the global namespace for any file that includes the header).4. At some point you might want to consider moving away from raw pointers in favor of RAII pointer containers (such as boost::shared_ptr or boost::ptr_map).   ;  Yes, supplying your own comparison function is important if you're using C-style strings.However, it could be that your problem is that you're not taking the lifetime/memory management of these strings into account.You get the string address from the XML file with:char *item = (char *)Pxml->getAttributeValue("itemName");But how do you know that this address will continue to point to the string forever? I'm not familiar with the IrrXMLReader, but it could be that it is recycling the memory that your pointer is pointing to.When you use std::string's instead of C-style strings, you are copying the string into your own memory, so your program will continue to work even if the original XML string disappears.   ;Quote:Original post by jykAlthough it would probably be wise to use string here, you should actually be able to use char*'s as in your posted example. It may be that something is going wrong elsewhere, but I can't really be specific (mostly because I'm not familiar with the irrXML interface).A few unrelated suggestions:1. Don't use leading underscores in your header guards (symbols starting with an underscore followed by a capital letter are reserved for the implementation).2. Using typedef's where appropriate will make your code more clear and readable (as well as more flexible/generic).3. Don't put using directives or declarations in header files (they pollute the global namespace for any file that includes the header).4. At some point you might want to consider moving away from raw pointers in favor of RAII pointer containers (such as boost::shared_ptr or boost::ptr_map).Thanks for the tips.  We're new to C++ and still have a ton to learn.  I lost the battle on Boost libraries, apparently they were thought to be too powerful and therefore overly complicated to include in the game we're making.   ;Quote:Original post by raebQuote:Original post by SiCraneUsing char * as a key doesn't do what you want. Switch to std::string instead.Ahh thanks a ton.  I was under the impression that if I supplied the map with way to compare character pointers that it would work ok.  Do you know why it doesn't work like that?  I'm just curious.Because a char*, being a pointer, is the value that represents where the string data is, not what it is. And if you have two copies of the same string data, they must be in different places (otherwise they wouldn't be copies). Thus the char* variables referring to them would compare as "different", even though the strings are "the same".Quote:Original post by raebThanks for the tips.  We're new to C++ and still have a ton to learn.  I lost the battle on Boost libraries, apparently they were thought to be too powerful and therefore overly complicated to include in the game we're making.Your teammates need to understand, that (even if you're not talking about C++, or even programming): (a) there is really no such thing as "too powerful" in the sense that it's meant here; there is only "has undesired side effects" (in the case of library code in programming, the most obvious undesired side effect would be performance or memory overhead, but Boost is carefully designed to avoid that except where it's actually necessary to solve the general problem).(b) "power" has nothing to do with "complexity".   ;Quote:Because a char*, being a pointer, is the value that represents where the string data is, not what it is. And if you have two copies of the same string data, they must be in different places (otherwise they wouldn't be copies). Thus the char* variables referring to them would compare as "different", even though the strings are "the same".Hm, I can't help but think that maybe folks aren't actually looking at the OP's posted code. The OP is in fact using a custom comparator (as in the example presented here); as mentioned previously, it looks like the problem most likely has to do with improper use of the irrXML API and/or a misunderstanding regarding the lifetime of the underlying string data, rather than a misuse of std::map.(Then again, maybe I'm the one who's misreading...)   ;Quote:Original post by jykHm, I can't help but think that maybe folks aren't actually looking at the OP's posted code.I think that's only Zahlman. Everyone else seems to have correctly identified the source of the bug as being:char *item = (char *)Pxml->getAttributeValue("itemName");gameItems[item] = new CGameItem();gameItems[item]->itemName = (char *)Pxml->getAttributeValue("itemName");The brain-dead irrXML documentation does not specify the ownership of the returned value or explain how to destroy it, and the returned string is a constant, which leads me to believe that the ownership is in fact retained by the parser, so it should not be stored, nor relied upon, beyond the statement in which it was generated.   ;  You could keep the char * if you changed the add element code to be:char *item = strdup(Pxml->getAttributeValue("itemName"));gameItems[item] = new CGameItem();gameItems[item]->itemName = item;Of course, then your program would have memory leaks without additional code to clean things up. Using a class, such as std::string, that handles allocating and freeing memory would take care of that.   