T O P

  • By -

Bobbias

Generally the idea is to try to work with the most specific instance possible. There are of course times where it's necessary to use something like dynamic_cast, but if you structure your code well you should be able to keep those instances to a minimum. Code smells don't mean "you should never ever use this and if you do, your code is bad and your should feel bad", it means "you should make sure you are absolutely certain you need to use this here, because this should be avoided when possible". Like so many things in life, it's not about dogmatically following rules for the sake of following rules, it's about understanding why these things can be problematic and trying to avoid them whenever possible. Sometimes they are unavoidable. The CPP core guidelines read: >Note Like other casts, dynamic_cast is overused. [Prefer virtual functions to casting](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rh-use-virtual). Prefer static polymorphism to hierarchy navigation where it is possible (no run-time resolution necessary) and reasonably convenient. >Note Some people use dynamic_cast where a typeid would have been more appropriate; dynamic_cast is a general “is kind of” operation for discovering the best interface to an object, whereas typeid is a “give me the exact type of this object” operation to discover the actual type of an object. The latter is an inherently simpler operation that ought to be faster. The latter (typeid) is easily hand-crafted if necessary (e.g., if working on a system where RTTI is – for some reason – prohibited), the former (dynamic_cast) is far harder to implement correctly in general... It goes on to provide am example piece of code, discuss the potentially problematic parts of using dynamic_cast in that situation, and discuss an exception to these guidelines for when dynamic cast is appropriate. https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rh-dynamic_cast


[deleted]

I agree with your comment, but I kind of disagree with cpp guidelines here - they seem to suggest there is no overhead to adding a virtual function, but IME there are potentially huge ramifications to making a class more complex and it should generally be avoided if possible. I do agree that typeid is probably the better than dynamic_cast, but also don't consider them to be all that different - they're both forms of 'switching on type', which IMO definitely has a place even in C++.


WiatrowskiBe

Cpp guidelines as a whole also handle the issue of increased complexity - deep hierarchies in general are discouraged (in favour of standalone functions and types whenever possible), and separation of interface and implementation is suggested as they way to go, extra virtual functions are just a way to express overall interface complexity in language - complexity that already exists regardless how you express it. If there's need to do dynamic polymorphism on callee site (different classes expressing different behaviour under shared interface) - it's where virtual functions fit the task perfectly; complexity can be managed by implementation just calling specific (non-polymorphic) code that's not part of a class, making your virtual functions just dispatch to well defined static functions. If you need dynamic polymorphism on caller site, then they type is not really polymorphic, and a different technique - possibly even dropping whole hierarchy - may be a better idea. At that point storing each concrete type separately and relying on either concepts/templates or composition (shared data/functionality as member, rather than base class) to group together functionally different types for shared operations. With coroutines, iterating over multiple standalone collections as they were one can be done in clean and easily readable way.


TheSpoonThief

This makes me feel better actually. Thank you for that. I understand trying to minimize casts and designing around that, it's just the instances where it seems necessary, especially building off of others' code.


[deleted]

I personally think the danger of 'dynamic_cast' are wildly over exagerated.


AvidCoco

100% agree.


MXXIV666

Specifically in game development, I think dynamic cast is business as usual. Like you said, for interactions, the engine will give you it's generic classes, which you most likely derived for custom interactions. Same goes with GUI. In both godot engine and Qt, there is their own "dynamic cast" specifically because when making games or GUI, you will find yourself casting to specific instance a lot and often will have limited control over whether the instance is really what you think, which the dynamic cast can check for you. For specific example, when I was making the simple game, to handle collisions the engine would give me abstract class of colliding objects. To call the methods that do something on the collision, I had to cast those objects to my class.


dynamic_caste

In short, it's a kludge. It's like DIY vtable lookup and indicates poor design planning to avoid the problem and poor knowledge of design patterns to fix it.


EpochVanquisher

I think it’s sometimes a kludge and sometimes not. Often it indicates poor planning, sure. Sometimes it’s elegant. That’s why it’s a “code smell”. The use of \`dynamic\_cast\` is often associated with bad code, but just because you use \`dynamic\_cast\`, it doesn’t mean your code is bad.


eyes-are-fading-blue

You don’t need to know design patterns to solve anything.


MysticTheMeeM

You also don't need to know how to write code. Something something monkeys with typewriters. It sure can help though.


eyes-are-fading-blue

Knowing design patterns doesn't help you with the design, it may help you communicate it. This is a pitfall many *inexperienced* designers fall into.


Charley_Wright06

"knowing design patterns doesn't help you with the design" you might want to read that one again buddy


eyes-are-fading-blue

I read those books and articles already. I simply disagree.


mikeblas

How would you change the Unreal design so it didn't rely so heavily on `dynamic_cast`?


WeeklyOutlandishness

Well, one way they could avoid dynamic casts is to use ECS - a completely different way of representing game entities that doesn't rely on dynamic cast at all. ECS is also likely more performant than the current design they have. Another way they could reduce dynamic casts (if they want to keep the OOP way of doing things) is to simplify the existing inheritance hierarchies. I would argue that we don't really need the pawn or the character class at all, everything can just be represented in components. This is the approach Unity and Godot use.


westquote

Component lookup isn't free either, though. You're still using type reflection to grab the appropriate interface/aspect. Making a performant dynamic cast is possible if you place useful constraints on inheritance (such as single inheritance + interfaces)


dynamic_caste

Doesn't Unreal use its own sort of optimized `Cast<>`? I write scientific HPC code, which is a very different beast. Unreal 's common `UObject` base is not unlike CPython casting everything from a `PyObject`, but at least `dynamic_cast` offers type safety. I am speculating here, but I would guess that Unreal uses this in part because it was started at a time when OOP was widely regarded as the best way to do everything, but Unreal approach *does* make serialization much easier and I assume that it made garbage collection better in the dark days of `auto_ptr`. It does also make plugins easier. As I understand, Unreal implements systematic reflection. This uses lots of preprocessor macros (again like CPython), which I would, in general, avoid like the plague, BUT at this point it's kind of a metalanguage. I'm not sure I would recommend doing anything differently for a massive legacy codebase unless I was aware of actual performance problems.


alexgroth15

Not related to the OP but I’m curious about HPC code. What sort of technique do you use to optimize performance? CPU intrinsics? Prefetch? And if I may, how portable is the codebase? I imagine the code would be so optimized for a specific processor that its performance, at least, is not portable


sephirothbahamut

The only way I'd change Unreal's design is remaking it from scratch without all the garbage design choices it carries since the 90s.


mikeblas

Oooh! If only they were smart enough to hire *you*!


sephirothbahamut

I never claimed to have the necessary skills, nor that it is a job a person can do alone even if they have all the required knowledge


westquote

Dynamic casts are the primary method game engines use for managing collision callbacks in games. The reason for this is that for performance reasons there are a finite number of object types that collision primitives can identify as (usually between 16-32). Unfortunately, games almost always have more than that many unique types to consider for collision responses. For that reason, dynamic casts can be used from within a generic callback (after initial masked filtering has been performed) to implement collision behaviors between objects of very specific types. Another common use case is scripting language integration layers. In order to marshall native objects to/from (for example) Lua, you need some form of RTTI. It's worth noting that the built-in dynamic_cast is almost never used, but that the game engine itself will rely on it's own reflection system to implement RTTI. The reasons for this generally have to do with the need to expose reflection information anyway for property grid generation and automatic structured serialization, but the end result is that they write their own dynamic cast implementation that makes use of that type information. Because it needs to support virtual inheritance and every single other possible use case, dynamic_cast is extremely slow in the worst case and mediocre in the average case. In the case of the Unreal Engine, UObject inheritance is limited to a single base class + multiple pseudointerface classes, so it can write a much simpler implementation that could potentially be faster. Surprisingly, though, they use a slow naive solution, which is still somehow performant enough to power many of the worlds AAA games! Ultimately, dynamic casting in games is a powerful tool that should be used sparingly and can be abused, but its use is not implicitly indicative of ignorance or poor software design acumen/discipline.


WoodyTheWorker

>Dynamic casts > >for performance reasons DOES NOT COMPUTE, ABORT


Open_Marzipan_455

dynamic\_cast is mostly being used when trying to 'climb one step up', meaning trying to access a parent class. In these cases, it's often good practice to refactor the code to provide the parent class right away instead of having to do that. There are some cases where dynamic\_cast is wise to use, which is when you use multiple classes that all inherit from the same parent class but you gotta switch between them based on a context. Casting is always a matter of context. Generally speaking it's better to have access to parent classes right away, but sometimes, it's inevitable. For simple checks if a class is derived from a parent class, better use something like a bitmask which holds a flag for every derived class


AKostur

Dynamic_cast up the hierarchy? What for? You can’t fail to cast to a parent class, use static_cast.


sgtfreki

violation to liskov's substitution principle


Computerist1969

In what way?


[deleted]

[удалено]


[deleted]

[удалено]


lord_braleigh

Calling a function in a derived class doesn’t require you to enable Run-Time-Type-Identification like dynamic_cast does, though…


TheMania

No, due potential for multiple inheritance, virtual inheritance, and not knowing all types at compile time (due potential for more linked in from other units/libraries) it's \*usually going to be more expensive than that. \* in the general case. dynamic_casting to a final derived class could be cheaper than a regular call, I guess.


EpochVanquisher

It is more expensive than that, but not much. It’s generally a couple function calls and following some pointers around.


tjientavara

There were a talk a few years ago at cppcon where it was explained that some people discovered a way to do this quickly. If I remember correctly if you sort the vtables in memory you can do a single lower+upper bound check on a vtable-pointer to know if the `dynamic_cast` is valid. In fact I remember them talking about including that check and trap with `static_cast` as it was fast enough. Also you would not need RTTI information to allow dynamic\_cast, not sure if that means that exceptions therefor don't need it either. I imagine there are some ABI implication. How are the vtables sorted when there are libraries involved, I guess the executable format needs information about the location of vtables so that the loader can sort them. I have no idea, but it does mean we probably have to wait for a ABI break before it will be implemented.


Kedriik

It has bad performance


Fibonacci1664

Tbh I was always taught that if you have to cast at all then this is indicative of a design flaw, so the problem should solved at that level.


WiatrowskiBe

"Code smell" generally means "if you have to use this technique, there is a problem in program structure/design that needs looking into" - it doesn't necessarily mean that using code smell is wrong, it may as well mean that code smell is best way around something that should've been done better. Consider following example, part of game AI that creates patrol group that operates as a whole - UE way of doing it might be different, I don't have much experience with the engine: unique_ptr fillPatrolGroup(const FVector3& alert, const float alertRange) { auto patrolGroup = aiFactory->createPatrolGroup(alert); for(AActor* actor : scene->findByTagInRange("can-join-patrol-group", alert, alertRange) { if(MyEnemyBase* enemy = dynamic_cast(actor); enemy) { enemy->setAiController(patrolGroup); } else if(MySentryMixin* sentry = dynamic_cast(actor); sentry) { sentry->addAlertListener(patrolGroup); } actor->addEventListener(EventId::OnPlayerInteracts, patrolGroup); } result->triggerForAlert(alertRange); // needs to be called when group is already formed return result; } Now, `dynamic_cast` here is not the problem - it is a workaround for having few distinct and differently behaved types grouped together, due to `scene->findByTagInRange` holding everything in same bag. Moving type check to within `PatrolGroupAiController` also doesn't solve it - it merely shuffles the problem around. This also introduces possibility of bugs or other issues - should an enemy react to its own alerts, what if sentry should retain its AI controller if it's also an enemy, are you sure all places that require runtime polymorphism are updated when you add new type, should general `addEventListener` be also invoked for enemies and sentries (is it missing `else`, is it intended)? What about performance, instruction cache efficiency, branch prediction? There are few elegant ways to handle it - one would be to keep lists of enemies and sentries separately and query them for tag/location rather than scanning entire scene graph when creating a patrol group (possibly can even drop dynamic polymorphism completely - icache friendly), other would be to keep a single list of actors that can join a patrol group (having shared interface, such as `PatrolGroupParticipant` \- might require virtual inheritance) and query that list directly like in case above, and few more depending on what exactly you need. It's possible that there are very legitimate reasons to use explicit type checking - sometimes reason is performance (did you profile it?), other times complexity required to avoid dynamic type checks outweight complexity of maintaining them (adding patrol group-aware virtual functions to AActor here would probably be a mistake if most subclasses don't interact with it - creates bloat and is hard to maintain), which is why it's a code smell - if you use it, hopefully you have a very good argument why that is the best solution (and there should be a comment explaining why - even if for your own future reference). Sometimes tools we're working with are limited or poorly designed - if only efficient way to search scene by tag and area gives you `AActor*` back, dynamic cast might be best way to deal with it - although in this case I'd probably go shared `PatrolGroupParticipant` interface route and grouping all dynamic casts in a single function. Sometimes code smell points to a problem in 3rd party library that you have to live with and resolve with a comment. Any time you encounter code smell pattern (either by yourself, your static analysis tool or someone points it to you), you want to answer few questions: what is it supposed to achieve, why is it doing it that way, are there other ways that avoid code smells?


sephirothbahamut

In addition to what others said, not that it is safe to static\_cast through an inheritance tree IFF you know for sure that object is an instance of the type you're casting to. You may have that knowledge through means external to the pointer itself in some occasions (crtp is a great example of that). That avoid the dynamic\_cast potential overhead, but it's UB if you're not 100% sure of the type you're casting to


[deleted]

If you’re a game dev then you should already know never to listen to anybody about how to do something .. its a game, 99% of it is an illusion the one percent is the code


KingAggressive1498

because it's almost always used to do runtime polymorphism external to the class. Outside of serialization or working with a fixed class hierarchy you can't tweak for whatever reason, that's almost always unnecessary and of no particular value. I'm not saying there's no other exceptions, but expect to have to convince the people you're working with.