Getting rid of nasty typeid()

Started by
25 comments, last by Valere 15 years, 6 months ago
I got a basic physics engine working. It's a 2D physics engine, currently only supporting AABBs and Spheres. Let me get to the point: I have the following classes: Entity: Controls forces and position. Has the following important functions:
void HandleCollision(Entity* pEntity, ShtrVector2 *pCollisionPoint, ShtrVector2 *pCollisionNormal)
This function only changes forces and position on itself, not on the entity passed through the function, the other entity is just used for retrieving mass/bounciness variables
virtual Update(float fTimeFactor)
Update position and acceleration through forces Collidable: Is a collidable object. Important functions:
virtual bool DoesCollide(ShtrCollidable *pCollideable)
simple virtual functions, when called directly through a collidable object, returns false, to prevent errors.
virtual void CreateBoundingSphere
when called on collidable object, destroys the current bounding sphere and sets it to NULL to prevent errors. CollidableAABB (public Collidable, public Entity): Implements DoesCollide and CalculateBoundingSphere further. Also implements update function so it also moves it's AABB CollidableSphere (public Collidable, public Entity): Same as CollidableAABB, just with a sphere My problem Inside the DoesCollide function, I have some very nasty typeid's. This is the code (this is the one for the AABB):
bool ShtrCollidableAABB::DoesCollide(ShtrCollidable *pCollidable)
{
	if (GetBoundingSphere()->Intersect(*pCollidable->GetBoundingSphere())) {
		if (typeid(*pCollidable) == typeid(ShtrCollidableAABB)) {
			ShtrCollidableAABB *pDummy = dynamic_cast<ShtrCollidableAABB*> (pCollidable);

			if (this->GetAABB()->Intersect(*pDummy->GetAABB())) {
				return true;
			}
		} else if (typeid(*pCollidable) == typeid(ShtrCollidableSphere)) {
			ShtrCollidableSphere *pDummy = dynamic_cast<ShtrCollidableSphere*> (pCollidable);

			if (this->GetAABB()->Intersect(*pDummy->GetSphere())) {
				return true;
			}
		}
	}

	return false;
}
Is there a way to get rid of these typeid's with polymorphism and good base classes (I haven't got a clue how to do this)? Or is this the only way to go? Thanks in advance, Max Henger
Advertisement
Why not just give the base class an enum Type variable that you can test instead of the typeid() / dynamic_cast, and have the derived classes constructor set that type (Or better, pass it as a parameter to the constructor of the base class)
This doesn't really belong in Math and Physics. Moved to General Programming.

In any case, as long as you're doing the dynamic_casts you don't need the typeids. Just check the result of the cast for nullness.
Quote:Original post by Evil Steve
Why not just give the base class an enum Type variable that you can test instead of the typeid() / dynamic_cast, and have the derived classes constructor set that type (Or better, pass it as a parameter to the constructor of the base class)


This is quite literally a textbook example of what not to do. First off, you've just replicated functionality provided by the language: you've reimplemented typeid. Secondly, even if you have a good reason for rolling your own RTTI facilities, adding a member variable to keep track of things is a waste of space and processing power. If you have to do this, make the type code come from a virtual function. This means that the type code is bound to the type of the object rather than the state of the object. Thirdly, that still won't eliminate the dynamic_cast, since the action that occurs is dependent on the concrete type of the object.
Quote:Original post by SiCrane
This is quite literally a textbook example of what not to do. First off, you've just replicated functionality provided by the language: you've reimplemented typeid. Secondly, even if you have a good reason for rolling your own RTTI facilities, adding a member variable to keep track of things is a waste of space and processing power. If you have to do this, make the type code come from a virtual function. This means that the type code is bound to the type of the object rather than the state of the object. Thirdly, that still won't eliminate the dynamic_cast, since the action that occurs is dependent on the concrete type of the object.
My reasoning for not using a virtual function was to avoid the overhead (Admittedly, it's a tiny cost compared to the actual collision test). The dynamic_cast could be turned into a reinterpret_cast, since the type is then known (Assuming the type is correct of course).

If this is the only place that RTTI is used, then I'd see my method as a perfectly acceptable solution, since RTTI can be disabled which can reduce code size (As I believe every struct and class in the application has RTTI information generated for it). The extra 4 bytes (Assuming the enum is 4 bytes) per instance is unlikely to be a significant memory increase.

EDIT: Hmm, too many brackets in this post [smile]
Quote:Original post by SiCrane
Quote:Original post by Evil Steve
Why not just give the base class an enum Type variable that you can test instead of the typeid() / dynamic_cast, and have the derived classes constructor set that type (Or better, pass it as a parameter to the constructor of the base class)


This is quite literally a textbook example of what not to do. First off, you've just replicated functionality provided by the language: you've reimplemented typeid. Secondly, even if you have a good reason for rolling your own RTTI facilities, adding a member variable to keep track of things is a waste of space and processing power. If you have to do this, make the type code come from a virtual function. This means that the type code is bound to the type of the object rather than the state of the object. Thirdly, that still won't eliminate the dynamic_cast, since the action that occurs is dependent on the concrete type of the object.



@SiCrane: RTTI isn't free... the compiler must generate code to keep track of the concrete type also, so I'm not sure why you said "adding a member variable to keep track of things is a waste of space and processing power". Further, the virtual function (which is basically how RTTI works anyway) requires a vtable entry and at least one call-through-a-pointer to operate. This requires even *more* processing power and storage.

What we usually do is:

1. Disable language RTTI in the compiler settings (which means we don't use dynamic_cast or typeid() at all).

2. When needed, add a type member variable that is initialized in constructors to the derived concrete type.


Now, having said all of that:

@OP:

You're ultimately trying to intersect the AABB of "this" with either the sphere or AABB of pCollidable. May I suggest a wrapper class that can contain the AABB and/or the sphere? Then you wouldn't have to check the type of the collider at all; instead you could inspect the contents of the wrapper inside the Intersect() function. That's one way... there are others. You're not stuck with the factoring you came up with.
Quote:Original post by Evil Steve
The dynamic_cast could be turned into a reinterpret_cast, since the type is then known (Assuming the type is correct of course).

Also a bad idea. A reinterpret_cast would reinterpret the bit pattern of the pointer as the cast type. However, that would not necessarily do the correct thing under multiple or virtual inheritance. If you must do this without a dynamic_cast at least use a static_cast which will at least give you some sort of warning if your type hierarchy would give you those kinds of problems. Or better yet, a boost::checked_cast which will use a dynamic_cast in debug and a static_cast in release.
Quote:Original post by mumblyjoe
@SiCrane: RTTI isn't free... the compiler must generate code to keep track of the concrete type also, so I'm not sure why you said "adding a member variable to keep track of things is a waste of space and processing power". Further, the virtual function (which is basically how RTTI works anyway) requires a vtable entry and at least one call-through-a-pointer to operate. This requires even *more* processing power and storage.

The compiler doesn't generate code to keep track of the concrete type, at least not any additional code over what it already does: the concrete type is implicit in the value of the pointer to the vtable, which is automatically assigned in the constructor. If you put a member variable that tracks a type into a class you've added an additional member to every instance of that class. This information is completely redundant with the pointer to the vtable. Setting this member variable is a waste of processing power because this action duplicates the action the constructor already does when it assigns that vtable pointer. Using a virtual function allows you to forgo this duplication of effort. Yes, using a virtual function is more expensive than reading a simple variable. However, by increasing the size of an object you decrease spatial locality for all uses of that object. Therefore, unless you are attempting to optimize the extremely unusual situation that you're doing type look ups more frequently than actually using the object, a virtual function lookup would win out in terms of actual overall program performance. (With some compilers you can even go an extra step and compare the vtable pointer directly against the vtable address and get your type information just as fast but without needing to bloat your class size.)

So again: adding a member variable to track the concrete type of an object is a bad idea because you're duplicating work that the compiler will be more than happy to do for you. This means that you're now in charge of maintaining that type information without help from the compiler. There will be no warning if you do something like assign two classes the same type id by accident or screw up by using the wrong kind of pointer cast to get your pointer.

Sometimes this headache will be worth it. Then you can disable RTTI and introduce your type code. However, using a member variable for that type code is a bad idea, since you've now duplicated the effort from the only place that the compiler can help you with your custom RTTI: assigning the vtable pointer. Not only is this duplication of effort apparent in the extra work you have to do, but also literally duplicates code and storage that the compiler uses anyways to do its work for non-RTTI purposes.
Sounds like you want to do double dispatch, which is pretty tricky to do in Cpp. The wikipedia article linked has one example of how it can be done (and it's even about collision!), but I suspect it doesn't quite fit the architecture you have in mind.

double dispatch, or explicit types. For the second case, I'd advocate a table of function pointers. Double Dispatch is nice, but the code can bloat very quickly.

Everything is better with Metal.

This topic is closed to new replies.

Advertisement