Jump to content
  • Advertisement
Sign in to follow this  
Funkymunky

C++ unordered_map copy/move constructor key location

This topic is 2141 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hopefully I can clarify things a bit after that title.  I have an unordered_map that uses std::string's as keys to store objects of a class called "Obj".  I want Obj to know what key it is attached to.  But I don't want to duplicate the std::string.  Ideally, that string would exist in one location; the key in the unordered_map.  Obj would just have a pointer to that.
 
So I have this:

 

class Obj
{
public:
   std::string *key;
};
std::unordered_map<std::string, Obj*> map;
Obj *GetMapEntry(std::string key)
{
   Obj *obj;
   std::unordered_map<std::string,Obj*>::iterator it;
   it = map.find(key);
   if(it != map.end())
   {
      obj = it->second;
   }
   else
   {
      obj = new Obj();
      std::pair<std::unordered_map<std::string,Obj*>::iterator, bool> entry = map.insert(std::pair<std::string,Obj*>(key, obj));
      obj->key = (std::string *)&entry.first->first;
   }

   return obj;
}

This seems to work just fine, presumably because I'm having the insertion use a copy constructor rather than a move constructor.  But I am a little skeptical that the unordered_map isn't going to get readjusted at some point and the pointer I have in the Obj class will no longer be valid.

 

Can someone chime in on this and tell me if this looks alright?  If there's a better way to do this, perhaps storing a pointer to the iterator instead?  My basic requirements are that the object class store a pointer that lets it manually force it's own removal from the unordered_map.

 

Share this post


Link to post
Share on other sites
Advertisement
Pointers and references to elements in an unordered_map are only invalidated by erasing the element in question. Storing an iterator to the element wouldn't be safe as rehashing invalidates iterators. However, your specific pointer usage is unsafe as the first element of the value_type for a std::unordered_map<std::string, Obj *> is const std::string, and you're storing a pointer to a non-const std::string.

Share this post


Link to post
Share on other sites

so I should make it

class Obj
{
   public:
   const std::string *key;
};

and then I don't have to cast when I assign the pointer to the key.

 

So, unless I'm mistaken, I've isolated the storage of the key string.  I can arbitrarily call "obj = GetMapEntry("some key string");", and it will only be stored in the unordered_map, and my object will have a pointer to it.

Share this post


Link to post
Share on other sites

Or you could use an unordered_set and specialize the hash and equality predicates. You still need to ensure that they key is not modified. Preferably by making it const and assigning it during construction of the object.

Edited by Washu

Share this post


Link to post
Share on other sites

Note that if you do that, std::unordered_set<Foo> stores const Foo objects. Not a big deal if Foo is a pointer to non-const since it will store a const pointer rather than a pointer to const, but if you store objects by value it may limit the operations you can perform on your objects. The other thing to note is that std::unordered_set<Foo> only does look ups based on Foo objects. So even if you only use a std::string to generate the hash, you still need to construct a full Foo object to look up objects based on that string.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!