CODEWITHSUNDEEP
c++pointersstldictionary
finlaybob
finlaybob

Reputation: 717

Can a C++ map contain a pointer to my arbitrary class?

I am writing a small game engine as a summer project, and am struggling a little with the STL map.

I have declared a class RenderList to hold objects. The RenderList will be passed to a Renderer class to do the work.

The RenderList has a map<std::string,Entity*> objects;

That all works, till I try to obtain an Entity* from the map and I get:

Assertion failed, in vc/include/xtree Expression: map/set iterator not dereferencable.

This is the code to retrieve the pointer.

Entity* RenderList::getByName(std::string str){
    return objects.find(str)->second;
}

I need it to hold a pointer and not the actual object, as there are different sub-classes of Entity which I'll need.

I am fairly new to the STL, should I not store pointers in a map?

Surely I should be allowed to do this, or is it a better idea to store objects instead?

And finally, am I simply doing it wrong!?

Hope this question is not a duplicate, I did do a quick search beforehand. Also if this would be better in the GameDev Stack I'll post it there.

Upvotes: 0

Views: 2487

Answers (5)

Jonathan Wakely
Jonathan Wakely

Reputation: 171363

If the key is not found then map::find(key) returns a "past-the-end" iterator, i.e. the value returned by map::end(), and that iterator doesn't point to any element so can't be dereferenced. You do not check what find returns before dereferencing it.

Are you sure they key is in the map?

You probably want to do something like return NULL if the key isn't found, which you can check for by comparing the returned iterator to end e.g.

Entity* RenderList::getByName(std::string str){
  map_type::iterator it = objects.find(str);
  if (it == objects.end())
    return NULL;
  return it->second;
}

Where RenderList defines the typedef:

typedef map<std::string,Entity*> map_type;

(N.B. I always make my classes define typedefs for the containers I use as implementation details, because it's much easier to write map_type::iterator than map<std::string,Entity*>::iterator and because if I change the container to something else I don't have to change all the code using it to e.g. map<std::string,shared_ptr<Entity>>::iterator I can just leave it as map_type::iterator and it still works perfectly.)

Regarding the general design, can you store a boost::shared_ptr<Entity> or std::tr1::shared_ptr<Entity> instead of a raw pointer? It will be much safer and simpler to manage the object lifetimes.

Upvotes: 8

AnT stands with Russia
AnT stands with Russia

Reputation: 320621

That probably means that the name you were looking for does not exist in the map. If the key does not exist, find method return the end iterator for the map, which is indeed not dereferencable.

If the "not found" situation is something that can naturally happen, then you can do this

Entity* RenderList::getByName(std::string str){
    map<std::string,Entity*>::iterator it = objects.find(str);
    return it != objects.end() ? it->second : NULL;
}

thus passing on the responsibility to handle this situation to the caller.

If the "not found" situation is not supposed to happen, either throw an exception or at least do 

assert(it != objects.end());

Upvotes: 3

Robᵩ
Robᵩ

Reputation: 168706

As others have pointed out, the name that you are looking up doesn't exist in the map. Everyone is quick to suggest that you check the return value of .find(). I suggest, instead, that you don't call .find(). Here is how I would solve your problem:

Entity* RenderList::getByName(std::string str){
  return objects[str];
}

In the code above, looking up a non-existent map entry will create an entry with a NULL pointer value, and return NULL.

You need to add some code somewhere to check for a null pointer before you deference it.

Upvotes: 1

Alex Wilson
Alex Wilson

Reputation: 6740

If the map does not contain the key you pass to the find method, then it will return objects.end(). Dereferencing this is a runtime error and will likely cause the error you see. Try instead:

map<std::string,Entity*>::iterator findIt;
findIt = objects.find(str);

if ( findIt != objects.end() )
{
    return findIt->second;
}
else
{
   // Handle error here
}

Upvotes: 1

Lou
Lou

Reputation: 1955

One thing wrong with this is that you need to handle the case where the there is no entry which matches str. Not sure what the specific error is about however as I've (regrettably) done the same dip into a map to retrieve a pointer..

Upvotes: 1

Related Questions