Old Style C Casts Not Recommended in C++?

According to the book I'm reading old style C casts are not recommended in C++. However, I've seen numerous examples of these casts in the Civilization 4 SDK. I would think that if their use was really discouraged then a program like Civ4 wouldn't use them, so is there any truth to this or is it just personal preference?

The old C casts still work, but the new ones are preferred as they force you to think "why" you are casting (const, up-casting, integer-size) and can lead to potential problems being discovered at coding time

Many programmers are stuck with "C+" thinking, that is, using C concepts within C++. Much is also legacy code.
Nevertheless, do not take after their example. Be a proper C++ programmer.

Casting in C++
In C++, you type cast an object if you want it to be treated as an object a different type.


When it comes to classes, a C++ object can be cast to either it's own type, or any other type it inherits. As the example in our code shows, since PlayerPaddle is derived from VisibleGameObject, it can be pointed to by a VisibleGameObject pointer. However, when it is pointed to by a VisibleGameObject, that pointer has no concept of an abilities beyond those of it's pointer type. Therefore to let it know that the VisibleGameObject pointer is actually pointing to a PlayerPaddle pointer, we type cast it.


Now you might be asking yourself, what happens if you cast an object to something it isn't? That’s a very good question with a very simple answer, with a standard cast, you just created a nasty bug in your program! This is why a number of different casts types exist.


First is the standard C style cast. int i = (int)someValue; is an example of C style cast. These are the most common types of casts, and potentially the most dangerous. Simply put, the compiler just assumes that what you are doing is perfectly logical. This means you can cast to types that aren't compatible. Even worse, you can actually cast a const object to a non-const object, generally another no no.


To rectify this potential hazard, C++ implemented a number of other cast styles. The one you saw in the example was the dynamic_cast<>. This handy cast actually checks to see if the cast is legit, and if it is not, it returns NULL. You may think that sounds wonderful, but there are a few catches. First off, dynamic_cast depends on RTTI being enabled to even work, which generally won't be a problem under most scenarios. Next, it is slower, which often can be a big problem! Finally, dynamic_cast can only be used on object pointers or references.


The next most common cast is the static_cast<>. It performs no runtime checks like the dynamic_cast so it is faster, but also it will allow an invalid cast, so it is not as safe. It however can be used to do casts that dynamic_cast<> cannot, like casting an enum to an int.


Next is const_cast<> when can be used to change the "constness" of a variable. Unless you have a very good reason, you generally shouldn't de-const anything! It is the only cast capable of doing this, except of course traditional C style casts.


Finally there is the reinterpert_cast<> when essentially allows you to cast things to things they simply aren't. For example, if you wanted to turn a pointer into an int, then later on back to a pointer you could use a reinterpert_cast<>. Unless you have a really really good excuse, and "look at this, isn't this cool!" is not a good excuse!, you really shouldn't use reinterpret_cast<>.


You can basically think of regular casts as capable of all of these different casts abilities, with all of their downsides too!




I pasted that from the casting section of my C++ tutorial. It doesn't directly answer your questions, but to understand why you would use one cast, you need to understand why other casting types exist.


Basically, the new casts give you less control, and generally this is a good thing. A C style cast is basically all of the casts rolled into one ( with the exception of RTTI info for example ). Basically, you could do anything, cast anything to anything, which frankly is dangerous as hell. Thus other casts came about.

If you don't need to have the ability to say... de-const your object, or change it to a type it doesn't actually support, you shouldn't have those abilities. The newer casts in C++ give you a more granular level of control

Thanks everyone for the replies. I'll take your advice and stay away from C style casts.

If you are performing so many casts that the manner in which you perform them is a question of some significance, perhaps you should first try to reduce the number of casts.

Both are fine if you know exactly what you are doing, and knows what's going on behind the scene.

Otherwise, C++ casts are safer.

Originally Posted by Serapth

As the example in our code shows

What code?

Originally Posted by DRK

What code?

Ahh, yeah, that would be the downside to cut-and-pasting that here, eh?


The chapter it came from is here:
Game From Scratch C++ Edition Part 7


The actual code was:

Code:

PlayerPaddle* player1 =      dynamic_cast<PlayerPaddle*>(Game::GetGameObjectManager().Get("Paddle1"));

I believe that was it.

I certainly hope that cast is only done at setup time and cached off for later. According to your code it appears that cast is happening at run-time during gameplay which is not good. Dynamic casts are very slow and if I recall are the slowest cast in C++. There are two optimizations you can do here and one is quite dangerous.

The first and safest one (from a cast standpoint) is to cache the pointer off at setup time so the paddle can be used later. The danger here is that it is possible other objects are using this pointer and thus could pull the rug out from under another object using the pointer. Using shared pointers would resolve this problem for you.

The second and least safe one is to use a static_cast there. Here is why I believe it is ok. There is a contract being formed here between the calling code and the container. The calling code is asking for Paddle1 so the calling code is assuming two things. It is assuming that Paddle1 does exist and that its type is PlayerPaddle. It also assumes that Paddle1 has been added and more than likely it was added by the calling code at some point. The API is merely providing functionality to the client code or calling code and is thus not ensuring type safety and leaving type safety up to the caller. Because Paddle1 was added by the calling code then it's type is also known by the calling code. So IMHO a static_cast here would suffice. Note that if Paddle1 is not of type PlayerPaddle and you use a static_cast bad things can and will happen when you attempt to call methods via the pointer. Also let's say the dynamic_cast failed. It will either fail because Get() returned a null in which case Paddle1 does not exist which points to an error in the setup code or it will fail because Paddle1 is not of type PlayerPaddle. If this is actually happening during run-time and gameplay as I suspect it is the game is DOA and nothing you can do will save it. You could check to see if the cast resulted in a null and all sorts of other mumbo jumbo but it will not change the fact that the game is going to come crashing down. Since a game normally uses OpenGL or Direct3D and there isn't a nice way to come down from those during a render I would say the added safety of dynamic_cast<> here is not needed and is wasteful.

The best option is most likely the first one I mentioned. It allows for setup code to fail and deal properly with the failure and it also allows the game to perform at a high level during runtime without the overhead of dynamic_cast occuring on each frame. You may think all that thought and performance mindedness is overkill in this code and perhaps it is but if you get into the habit of dynamic_cast'ing all objects returned from a heterogenous object container at run-time I guarantee you will run into serious performance problems at some point.

Originally Posted by VirtualAce

I certainly hope that cast is only done at setup time and cached off for later. According to your code it appears that cast is happening at run-time during gameplay which is not good. Dynamic casts are very slow and if I recall are the slowest cast in C++. There are two optimizations you can do here and one is quite dangerous.

The first and safest one (from a cast standpoint) is to cache the pointer off at setup time so the paddle can be used later. The danger here is that it is possible other objects are using this pointer and thus could pull the rug out from under another object using the pointer. Using shared pointers would resolve this problem for you.

The second and least safe one is to use a static_cast there. Here is why I believe it is ok. There is a contract being formed here between the calling code and the container. The calling code is asking for Paddle1 so the calling code is assuming two things. It is assuming that Paddle1 does exist and that its type is PlayerPaddle. It also assumes that Paddle1 has been added and more than likely it was added by the calling code at some point. The API is merely providing functionality to the client code or calling code and is thus not ensuring type safety and leaving type safety up to the caller. Because Paddle1 was added by the calling code then it's type is also known by the calling code. So IMHO a static_cast here would suffice. Note that if Paddle1 is not of type PlayerPaddle and you use a static_cast bad things can and will happen when you attempt to call methods via the pointer. Also let's say the dynamic_cast failed. It will either fail because Get() returned a null in which case Paddle1 does not exist which points to an error in the setup code or it will fail because Paddle1 is not of type PlayerPaddle. If this is actually happening during run-time and gameplay as I suspect it is the game is DOA and nothing you can do will save it. You could check to see if the cast resulted in a null and all sorts of other mumbo jumbo but it will not change the fact that the game is going to come crashing down. Since a game normally uses OpenGL or Direct3D and there isn't a nice way to come down from those during a render I would say the added safety of dynamic_cast<> here is not needed and is wasteful.

The best option is most likely the first one I mentioned. It allows for setup code to fail and deal properly with the failure and it also allows the game to perform at a high level during runtime without the overhead of dynamic_cast occuring on each frame. You may think all that thought and performance mindedness is overkill in this code and perhaps it is but if you get into the habit of dynamic_cast'ing all objects returned from a heterogenous object container at run-time I guarantee you will run into serious performance problems at some point.

The reality is, it was just demonstrative code. It points out elsewhere in the same section that a) failure isn't handled properly b) it is slow.

Perhaps the even more grave sin, there aren't actually any methods in the casted class that were required ( all methods used are actually available in the VisibleGameObject class ), so that cast isn't even required. I run into a few examples like this throughout the tutorial, I also introduce an assert at a point where it isn't really the best way to handle an error ( and mention that as well ), while at another location I instead use Exceptions ( again, so I can introduce the concept ).

So, I suppose what I am saying is, the code isn't meant as something to copy and paste, then take away and use as is, it is more a learning experience. I don't spend too much time coding around possible errors ( hardening ) or worrying about performance concerns, as I want to keep things somewhat focused. I point out the potential problems and the "real world" differences and move on. Often then, in a later chapter, previous warts are revisted and fixed once the reader has a bit more exposure and understanding why something is a wart in the first place. One such example is the audio system loads a sound from file every time it is played, which is obviously quite bad. Later on though, caching is introduced and this code is revisited. On the whole, by feedback I've received, this process seems to be working.



So, truth of the matter is, in this case, any cast is actually a waste . You are perfectly correct though, a dynamic_cast has a performance penalty, but even on a perframe basis, in a game as simple as pong, I don't really thing this is a detail a new user should focus on. Readability/functionality first, performance later. I agree with you completely in regards to the fact that a failed cast in this case is not something you can really recover from. At that point, "It's dead Jim" and it is time to wind now. Actually, looking back in time, this would have been a very good time to introduce the concept of asserts!

Sounds good. I did not want you to get into the bad habit of using dynamic cast's all over your game code at run-time and then wonder why the thing started to slow down later. The problem with some books and tutorials is they show one isolated piece of code and then the unwary use said piece of code in a much larger system that has for more constraints and eventually the code no longer works as intended. The important concept is for a game is always pre-cache your data and your objects if possible. Get the types correct at setup time or load time so you can leave as many cycles as possible for rendering and game logic. But for pong this is super overkill....but there is never anything bad about developing good habits early even if it is for Tic Tac Toe.

Very true. The only downside to caching of pointers is if the object you cached expires. This is why auto_ptr and it's kin are so incredibly powerful ( and a topic I need to cover at some point! ).

You are correct though, good habits are extremely important, especially early on. Actually, that was a big part of the reason I developed that tutorial series in the first place, as so much of what's out there is absolutely awful and teaches some downright terrible coding practices. Things like const correctness seem to be virtually unheard of in tutorials, and you often see appalling things like people recommending DevC++ or mixing new and malloc. Also, thank you for pointing out the flaw, another set of eyes is always appreciated. I have been using this language off and on since, well, since it was created and still get tripped up all the time! ( It also doesn't help that I basically switched 99% of my coding over to C# since it was released ).




// Unrelated, I HATE!!! HATE!! HATE!!! that blue Reply to Thread button... I've lost like 6 replies to that stupid button!

This is why auto_ptr ....

Beware of auto_ptr's weird copy semantics. I honestly only see value in auto_ptr when used in a constructor so that if allocation fails at some point in the constructor previous allocations will be cleaned up. Boost shared_ptr is a very good smart pointer that does not have the same issues as auto_ptr. To be fair I do not think auto_ptr was intended to be as robust as boost::shared_ptr.

auto_ptr is broken because its defects couldn't be solved with the language features at the time it was invented.
However, the new unique_ptr should fix those defects with the new C++11 standard.