typeid problem

Started by
5 comments, last by ekrax 19 years, 5 months ago
hello, im having a little trouble getting something to work, and i have no clue why it isnt working . . . becasue it should be working. anyways i have a member function which returns a pointer to a base class then i want to find out which class its pointing to by using typeid then push_back the proper vector. it looks like this. CObject *newObject = factory->getObject(sObjectName); if (typeid(newObject) == typeid(CFloor)) v_Floor.push_back(*(dynamic_cast < CFloor * > (newObject))); if (typeid(newObject == typeid(CScenery)) // . . . delete newObject; the problem is that its never pushing back the object, which means the type of newObject is never CFloor or the other object types i have. factory->getObject(sObjectName) will call this . . . _mCObject::iterator i; i = prototypes.find(key); CObject *newObject = new CObject (*(i->second)); if (i == prototypes.end()) // report error return newObject; i should note that _mCObject is typedef'ed as std::map < std::string, CObject * > and prototypes is a map of CObject pointers when i say // report error i basically have a function call there displaying a messge box and logging an error . . . which never happens so im not trying to get an invalid key. so im confused why this isnt working? what is wrong with this . . . if (typeid(*newObject) == typeid(someObject)); also i should note that factory->getObject(name) will return one of the objects im testing for. the typeid fails for all of the objects. anyways, if anyone could help me out i would appreiciate it. thanks in advance. [edit] CObject *newObject = new CObject (*(i->second)) . . . this line just struck me . . . im pretty sure this is right in that im initializing a newobject with the value pointed by the iterator i? [/edit] [edit] ok (*(i->second)) im pretty sure is correct but for some reason when *newObject is pointing to a floor object its name is 7CObject and CFloor is 6CFloor . . . first of all where are these numbers comming from? and the name of a base class pointer type should be of the class its pointing too shoudnt it? [/edit] [Edited by - ekrax on November 8, 2004 10:03:39 PM]
Advertisement
Quote:Original post by ekrax
hello, im having a little trouble getting something to work, and i have no clue why it isnt working . . . becasue it should be working. anyways i have a member function which returns a pointer to a base class then i want to find out which class its pointing to by using typeid then push_back the proper vector. it looks like this.

CObject *newObject = factory->getObject(sObjectName);
if (typeid(newObject) == typeid(CFloor))
v_Floor.push_back(*(dynamic_cast < CFloor * > (newObject)));
if (typeid(newObject == typeid(CScenery))
// . . .
delete newObject;

the problem is that its never pushing back the object, which means the type of newObject is never CFloor or the other object types i have.


Yeah thats because the type of newObject is of type CObject* which is never equal CFloor or CScenery, you need to dereference the pointer e.g.

if(typeid(*newObject) == typeid(CFloor))    //...


Quote:Original post by ekrax
factory->getObject(sObjectName) will call this . . .

_mCObject::iterator i;
i = prototypes.find(key);
CObject *newObject = new CObject (*(i->second));
if (i == prototypes.end()) // report error
return newObject;

i should note that _mCObject is typedef'ed as
std::map < std::string, CObject * >
and prototypes is a map of CObject pointers

when i say // report error i basically have a function call there
displaying a messge box and logging an error . . . which never
happens so im not trying to get an invalid key.


this line:

CObject *newObject = new CObject (*(i->second));


looks suspicious and the possiable reason why your type switch if statements are not hooking, you want create an instance which is a sub-type of CObject, your also most likely causing object sliciing doing that, you wont to change that code to something like:

CObject* factory::getObject(const std::string& key) const {   _mCObject::const_iterator i = prototypes.find(key);   if(i == prototypes.end())      return 0;   const CObject* const obj_ref = i->second;   if(key == "CFloor")      return new CFloor(*obj_ref);   else if(key == "CScenery")      return new CScenery(*obj_ref);   else if //etc, etc      //etc, etc   else //default case      return 0;}


this is really ugly coding & full of maintance nightmares & inflexibility.

The some other alternatives are:

A. apply the virtual constructor idiom to your hierarchy

adv:
cleaner than type switching,
use of co-variant returns,

cons: has it has some flaws too.
its intrusive,
new sub-type may forget to override there parent's version of virtual clone/create methods.

B. instead of having a map of some unique identifier to proto-typical instances, have a map of unique identifies to creation product meaning the creator could be a pointer to function or a generalized functor e.g.

template <   typename AbstractProduct, //<-- base type of all products   typename Key = std::string, //default key type is a string   typename ProductCreator =  AbstractProduct* (*)() //default creator type is a pointer to function   typename Comparator = std::less<Key>>struct factory {   typedef std::map< Key, ProductCreator, Comparator > fac_map; //really a hash map would be better.   //...private:   fac_map _fmap;public:   //.....};


only a slight problem with it is you need to register a product creator but its not that bad, and you have to change the above for cloneable factories.
ok i forgot the * in typeid(newObject) but it still didn't work when i did that. i also see what you mean how it is sloppy ... ie its an annoyance if i want to add more objects or take some away. so looks like im gonna do the virtual constructor idiom ... i still havent learnt it yet even from the last time you suggested it. but since its a reoccuring solution to my problems im just gonna hafta stop being stubborn and do it right. i find the second way you pointed out interesting aswell and i might try that out.

thanks for the reply.
Quote:Original post by ekrax
ok i forgot the * in typeid(newObject) but it still didn't work when i did that.


Yeah thats because of this line:

CObject *newObject = new CObject (*(i->second));


the type that newObject refers to is of type CObject so its never CFloor or CScenery.

Quote:Original post by ekrax
i find the second way you pointed out interesting aswell and i might try that out.


That is preferable, basically thats what loki library's factory does but it also has a cloning factory that uses proto-typical instances from what i remember so you might be intereseted in having a look at it, there really simple to understand.
ok i got the program working with the virtual constructors, but i still have to use the if else structure ...

CObject *newObject = (factory->getObject(name))->clone();
if (typeid(*newObject) == typeid(CFloor)) {
v_CFloor.push_back(*(dynamic_cast < CFloor * > (newObject)));
}
if (typeid(*newObject) == typeid(CScenery)) {
v_CScenery.push_back(*(dynamic_cast < CScenery * > (newObject)));
}

so im gonna try the other method aswell, as i shoudnt have to do this that way?
Quote:Original post by ekrax
ok i got the program working with the virtual constructors, but i still have to use the if else structure ...

CObject *newObject = (factory->getObject(name))->clone();
if (typeid(*newObject) == typeid(CFloor)) {
v_CFloor.push_back(*(dynamic_cast < CFloor * > (newObject)));
}
if (typeid(*newObject) == typeid(CScenery)) {
v_CScenery.push_back(*(dynamic_cast < CScenery * > (newObject)));
}

so im gonna try the other method aswell, as i shoudnt have to do this that way?


Well really you don't need to do a type check & a dynamic_cast, just try the cast, depending on wheather you cast to a pointer or reference the former will return 0 while the latter throws a bad_cast exception if isn't the the type your asking for so you only need to do a cast e.g.

if(const CFloor* const f_ptr = dynamic_cast<const CFloor* const>(newObject))   v_CFloor.push_back(*f_ptr);else if(const CScenery* const s_ptr = dynamic_cast<const CScenery* const>(newObject))   v_CScenery.push_back(*s_ptr);


But thinking about i'm sure what ever your trying to do there is a much better alternative, your making copies of that instance is there a particular reason? are you sure you need a factory just for that?

Don't you think it would be better to maintain containers of each concreate type but pass around a base (smart) pointer to them instead of creating an instance on the heap, returning a base pointer then doing a dynamic_cast back to its real type and then making a copy of it?

i might be wrong, its hard to say unless we know what it is your hoping to achieve.
the purpose:
ok i have a bunch of derived types from a base class, i want to store prototypes of all the objects i need in the game in a factory class, the objects are defined in a text file and loaded and created when the factory class is constructed. the factory class contains a map of base class pointers which hold all the objects. now i have a manager class which hence the name manages all game object instances, so when adding a game objects i would call this ... manager->addObject("tree1", 0, 0); which would then create a tree at world position (0,0) ... withen the addObject, as you already saw, a call to the factory is made and a request for the desired object is returned. this object is then created in memory and stored in each objects vector.

now you might be thinking, why not just put the instance of the object withen a base class pointer list? well i was orignally gonna do this, for one it would be simpliar and cleaner, but it also makes it more difficult to render everything. so it comes down to whats better ... and now that i think about it, i think i should just go with the list.

1) with the list, i have to sort the list every frame so it will render everything in correect order (all floor objects first, then scenery objects, then player objects, etc.)

2) with the vectors i never have to sort anything, but if i have 5 vectors of objects i have to iterate through each one.

i guess maybe the list would have been better, and using vectors of individual object types is worse. am i right in the assumption that using the vectors of individual types is unneeded?

This topic is closed to new replies.

Advertisement