Code Smells - 'Switches' in Unity3d / C# Architecture

[@albingrahn5576]

"spell" doesn't even sound like a real word anymore lol. great video though, i'm currently making a game where i plan to have a lot of spells/abilities and i would 100% have fallen into the switch statement trap if it weren't for this video, so thank you!

[@GameDevNerd]

How's your game project coming along? I wanted to let you in on a little enum secret that you may already know but if not it can help a lot. Suppose you have a spell that fits with two or more SpellTypes ... suppose it increases the target's armor or attack abilities but drains health or something. So what enum do you use? SpellType.Damage or SpellType.Ability? Or do you have to create a whole new one called SpellType.DamageWithAbility? That will get really messy, won't it? Luckily, .NET has the [Flags] attribute to stick on enums where you can combine values and check them like bit flags: [Flags] public enum SpellType { Default = 0x00, Damage = 0x01, Heal = 0x02, Ability = 0x04 }; You combine Flags with simple bitwise operations, in our case just like this: var specialType = SpellType.Damage | SpellType.Ability; Now it serves as both a damage and ability spell type so other code can read the Flags and see what it's for. In case you're wondering what is this 0x00 and 0x02 weirdness that's just hexadecimal notation for the numbers 0, 2 and 4 ... 0x0A would be 10, 0x0F would be 15 and 0x10 would be 16, hex is just simple base 16 numbers instead of base 10 (decimal) or base 2 (binary) or base 8 (octal). I use the hex notation so I immediately am reminded when looking at my code that I'm dealing with a bit flags enumerator. The reason for using the powers of 2 for each value constant is so there is no overlap in the bits. Those same numbers in binary would look like: 00000001, 00000010 and 00000100 ... So if the first bit is toggled it has a Damage effect, if the second bit is toggled it has Healing effects and if the third bit is toggled it has an Ability effect! 00000111 has all three! It's a pretty handy little trick for complex enums that may be some combination of something. 😄

[@7th_CAV_Trooper]

I've watched a couple of Jason's videos now and this one earned my sub. Pretty solid explanation of the switch anti-pattern and how to solve it with abstraction. I see junior devs write rule-based code with switches all the time. Pro tip: action maps > conditional statements. at 17:25 - "we cast it as a spell" lol. nice U can create a static constructor instead of having an Initialize method.

[@snikodive5077]

This video is one of the most interesting i have ever seen. You have found a very typical example (as you said code smells) and provide us professional solutions. It is a lot better than videos that want to explain design patterns. Keep going guy !

[@madmanga64]

Your channel is underrated

[@neerajkumar-greedgamesstud1259]

Yess

[@neerajkumar-greedgamesstud1259]

Yes because he make knowledge content not fun content like dani

[@r1pfake521]

The SpellType enum is usless after the refactoring, because the spell class (type) itself can be used instead. And you can just use a static constructor for initialize instead of the additional bool.

[@cuzsleepisthecousinofdeath]

Well, we can make a Dictionary, keep the enum, and get rid of the various inherited classes. Because why classes, if we only need one method to be executed.

[@cuzsleepisthecousinofdeath]

(Or maybe i'm not exatcly right, since i have a bias against very contrived examples, that's why i wasn't following the vid second to second)

[@NiclasGleesborg0]

Will definitely take your suggestions into account.

[@justinwhite2725]

More of this please! I’m self taught programmer so I don’t know any of the best practices. @1:54 omg! See, this is the kind of thing I need. I know this concept but I’ve been writing a getter without a set every time. This shorthand is fantastic!

[@gigajoules6636]

I would recommend Christopher okravi's code walk series.

[@naviseven5697]

Get the Design Patterns: Elements of Reusable Object-Oriented Software book. It's a must for every programmer.

[@Kormelev]

How about a code smells for when you check what scene is loaded in your code?

[@ZnelArtsGames]

Interesting, is an improvement over a switch but I would argue that using reflection is a code smell by itself, for this is more appropriate the Adapter Pattern.

[@andremilanimartin3338]

didnt quite get why switch would be bad, but learned a whole new bunch of editor short cuts and programming methods, so thanks anyway

[@cavalfou]

spellType Enum get rapidly confused with the Class Type of the spells, but your point is well proven.

[@tuckerwolfe]

I particularly enjoyed this video. Getting close to graduating and things like this really help me see a bigger picture. More code smells!

[@mattsponholz8350]

Brilliant! You created this at a freaking great time for me! I was just about to mull over some ideas for rewriting my AudioManager base packages; which, unfortunately, we’re pretty Emum/switch heavy when implementing... thank you!

[@sagiziv927]

I understand what you are trying to accomplish, but I think that this way (reflection) is not that good for it. What if your spells needed to get some unique parameters (e.g. how much mana this spell consumes from the character) it would be impossible to pass the value in the reflection. That's why I think that implementing it with ScriptableObjects is way more comfortable because then you can assign this values in the Inspector. And then put them inside a dictionary like you did. What do you think¿

[@legendaryitem2815]

I agree, that's a good option. Also, if you don't have any extra functionality (e.g the class just holds the spell's data) and you don't need to alter any of the information for the spell (e.g you set the parameters in the editor and don't change them at run time) then perhaps consider using a Struct to hold the spell information as it creates less overhead/garbage collection as they are passed by value rather than by reference.

[@caliche2000]

In order for that to happen you add an abstract method or property to the abstract class and it HAS to be implemented by each class inheriting from the spell abstract class.

[@Sylfa]

@@legendaryitem2815 Struct wouldn't help here, the dictionary is a reference type and would box the struct's up. In fact, you can't have an abstract struct so you would have to use an interface as a base and implement it in each struct. And then every time you did a lookup in the dictionary you would first unbox the struct, putting it as a copy on the stack, before you call into it's methods. It wouldn't even do much good if you remove the methods and put them in the SpellProcessor, first your breaking encapsulation. Secondly your recreating the switches. It's better since they are all in the same class, but still a code smell. And third your only saving a single pointer lookup. Since your never recreating these objects they will go into the old generation memory rather swiftly, there they will sit, not really effecting garbage collections much. You never recreate them since they are reference types, only passing the pointer along. And in the end your only doing 2 method calls per spell cast, that's 2 pointer follows, and not per frame but per player input which is much slower.

[@Sylfa]

"it would be impossible to pass the value in the reflection." Doing it this way each class describes that spell entirely, so if you have a resource usage or cooldown you would implement that either in ProcessOnCharacter (ie add caster to it and then manipulate the caster as well), or you would add an abstract property like @Carlos_Maya said. In that case you force each spell to record how much mana it uses, or how long it's cooldown is. I happen to think ScriptableObject is slightly better in this case, it allows you to drop the enum, making it so you only need to change 1 source file to add a new spell, or creating a new object for making a variation. The only time it feels awkward is if your not exposing any properties at all on your spells, making it so you only ever create a single ScriptableObject. This way you avoid the SpellProcessor lookup mechanic, and instead use the Unity editor like an inversion of control dependency injector which is a very nice feature on it's own, even if the database being files is a bit awkward at times.

[@georgimihalkov9678]

Reflection? Wouldn't it be a performance hit? Wouldn't it be easier if we at least cached the needed methods as delegates at runtime and just invoke them when needed?

[@georgimihalkov9678]

@Andre SCOS absolutely! I wanted to point out that reflection is not needed in this case.

[@georgimihalkov9678]

@Andre SCOS reflection should be used sparingly.

[@georgimihalkov9678]

@Andre SCOS ofc depends on the use case, but I prefer getting the job done without reflection if possible. Reflection can be up to 600 times slower than normal code invocation (it is more optimized in .NET 5/ Core). I agree that in this example you execute it one so it won't impact the performance.

[@owencoopersfx]

I have a simple personal project where I’m using several switches and have run into this exact problem... like having to change things in a bunch of places when I’m trying to add new functionality. I’ll need to rewatch this a few times but this is great to know about to improve the code. Thanks!

[@TheThormond]

Imo this abstraction is severely limiting. I would imagine in MOBA's/MMOs (or any other genres with complex mechanics) spells would have tags, not types. For example: what if you wanted to have a spell that would root an enemy and drain his life over 3 second period? That would be a root, a damaging spell and a heal (since you're draining) at the same time. Another set of problems comes from the fact that root also exists in some kind of hierarchy of Crowd Control effects, so you would have to group it up with into one category with Stun, Knockdown, Silence, etc (also how do those effects interact together, can a target be rooted and knocked down a the same time or is it on the same debuff slot? which one takes priority if its the same debuff slot?). Then the tag would have to be #CrowdControl, not #Root and you end up in a situation where it would be much better to avoid tags altogether and just do an Object approach. The way I'm usually doing it is having spells be "buckets" for effects that derive from some abstract Effect class. You would have a Spell class that has an array Effect[] and you would populate it with descendants of class Effect through polymorphism. Probably something along the lines of "Action" would be a better name for it since you could also apply it to attacks, character emotes and anything that actively launches some series of effects. Then at some point you would want to be able to launch those "procs" at certain animation thresholds (for example attack would apply damage at 0.4 animation point) so I guess that's where you want your structure with effects be called "Ability" (so you end up with Ability that inherits from Action and allows you to pick animation thresholds for each of the assigned Effects). Effects would be resolved independently by their respective effect processor (which most likely derives from an abstract EffectProcessor class).

[@gigajoules6636]

You create a new type for this combined behaviour and implement multiple health changes within method that actually implements damage in the concrete class

[@TheThormond]

@@gigajoules6636 and you will end up having to to hard-code every single ability which will cause massive overhead to prototyping. What im suggesting is that you keep Effects as seperate scriptable objects and attach them to Abilitiy scriptable objects (or whatever data structure you're using, as long as it has an editor interface accessable to the designers) and just have a system resolve the Effects as they are procced.

[@shenlong3879]

@@TheThormond As sad as it may sound a lot of established developers work that way because they just don't know any better. I think that's where an Entity Component System can work wonders. Generalizing and reusing small components and combining them for different effects. Also I honestly hate the idea of games differentiating what I can use offensive of defensive spells on etc. I have the spell, I want to cast it on something, let me deal with the consequences and don't artificially limit me.

[@1i1x]

Know any advanced tutorial channels like for MMO? I think this guy is just throwing something in there to make a video, not professional.

[@TheThormond]

@@1i1x I don't think there are. People who know state-of-the-art MMO architecture usually don't make KZbin content, they are too busy working on their MMO projects. In addition KZbin creators are biased towards solutions that enable fast prototyping which allow you to produce content fast and maintain viewerbase so I wouldn't count on their expertise in creating large code bases that can be extended and maintained for years, that requires a completely different mindset. Udemy/Skillshare is a better bet but I don't think you can find what you want there either.

[@Fralleee]

The whole initialization seems unecessary. Why not have the processor accept a "Spell" as parameter. This solution can't handle if you have multiple spells that are of the same spell type?

[@TheSpacecraftX]

Saw the title and looked nervously at the 2 Switches I used today for a 2D puzzle game. Determined my use case is absolutely fine though because my enum is fixed size of 3. AllignToInput, AllignToOutput, and None.

[@Krakkk]

Why won't you use ScriptleObjects for spells? Wouldn't it be cleaner?

[@Unity3dCollege]

It depends on the use case. Been doing a lot of mmo stuff again lately and in games where the server is so authoritative and data is constantly being changed on the fly, automatically updating on a server, and populating out to clients, scriptable objects aren't nearly as useful. But I completely agree that in the vast majority of cases they're a great way to store the data as well.

[@Krakkk]

@@Unity3dCollege Thanks. I didn't think about it.

[@ravanin]

@@Unity3dCollege given how many people are suggesting scriptable objects, maybe could you release an addendum video wit scriptable objects?

[@behzadbakhti]

Thanks for your video, I would like to learn about events , delegates, and specially callbacks in unity way, that would be great if you make some in-depth video about this stuff.

[@joaoferreira_yt]

I'm starting now on Unity and I migrated from Mobile Development. I'm glad that I actually did something like that except for the "Processor" class, that I already implemented. Cool tips, thanks!

[@The28studio]

This is great . Thanks Jason.

[@leokamikkaze20]

Jason is great. Thanks this.

[@Dxpress_]

Jason is this. Great thanks.

[@Unity3dCollege]

You're welcome! :) thx!

[@mattmurphy7030]

Code smell videos are a code smell

[@dougwarner59]

I never thought about using named parameters on a method having a single parameter; It does make it easier to see what is going on though.

[@abwuds7208]

Alright, when you're typing "case" you should just hit Ctrl + Caps + Space to trigger the smart completion! It works everywhere (when assigning something to a variable, passing a parameter, writing a switch/case...) Thanks for the video!

[@kutanarcanakgul5533]

If we have so many animations on our character, it can be turning a mess for sometimes. How are we organize our animations?

[@GameDevNerd]

Jason you should do a game architecture video about designing something like a spell and weapon system where you talk about the way classes will be structured amd GameObject hierarchies created rather than the specific graphics and special effects and things like that ... just the architecture and structure part. Will be very helpful to many of us who know how to do a lot of specific things but get confused about engineering large systems in complex games.

[@BehindTheFXTuts]

Can anyone tell me how i can get the "helper" in the script that says "Set By Unity" and "Scripting Component" etc...?

[@svetlinsvilenov8739]

This is a great video. I did a tutorial recently for a turn-based combat system, and the state machines were made with huge switches, so now I'm going through the project thinking of ways to refactor the thing :)

[@mattCap42]

Thanks for this. I definitely seem to be leaning too hard on switches. I have a loot manager and spell manager that are perfect use cases for this solution! I just finished scripting a boss fight where the boss stops fighting and summons adds at 25% health intervals. I have a method called at each of the three stage transitions with a switch that checks the fight stage and sends the stage appropriate array of adds to a spawn manager. This is a very narrow case and wouldn't be referenced anywhere else in the code. does this pass the smell test? or do you think it best I find another way to do it?

[@carlosvega4795]

I'm trying to make a mechanic where the player can craft their own spells (depending on the knowledge the character has gathered and the character stats themselves, thus some spells might be suicidal/impossible/expensive for some characters but way too easy/spammable for more gifted ones), let's call it a spell-forge, the same way you craft items in other RPGs, difference being you can not loot spell knowledge, unless the player is perceptive and smart they can not reverse-engineer spells used by AI wizards. Anyways, point is: I coded all that using Scriptable Objects... I didn't know anythng about abstract stuff... But be sure I'll be using that now thanks!!

[@MQNGameDev]

Great Video. Thanks for touching on a practical use case for reflection. Your the best.

[@parthpandya008]

Very good video for switch statement alternative. Love to see more regarding reflection.

[@bahtiyarozdere9303]

What about the "IsValidTarget()" function? I think it was not called at all after refactoring.

[@Arrow5587]

May be make a method in SpellProcessor to check a spells's validity before using the SpellProcessor's use spell method

[@БогданАльфавіцький-д3к]

How switch statement can be a bigger smell than damn reflection? Most of the time ScriptableObjects is the way to go. Sometimes switch can be the only option. But reflection? This is an absolute NO for me.

[@Unity3dCollege]

I'm curious why? What is it about the reflection that you're concerned about?

[@jaxowaraxa]

@@Unity3dCollege I think one of two: 1 performance issues 2 he doesn't understand reflection

[@БогданАльфавіцький-д3к]

@@Unity3dCollege As was said before by others, it's unstable. From platform to platform it may break. It's not an intended way to write game logic. For testing, custom editors, tools? Yes. For actual game code? No. And, it's hard to debug. There are a lot of cases where it can be used. But in my opinion Unity has tools to avoid using it.

[@KenderArg]

@@Unity3dCollege As rule of thumb never use reflection on your own code. It is for finding out things at runtime that cannot be known at compiletime. Ideal for plugins, mods, editor tools and other uses of 3rd party assemblies. In this case it's being used to find out about code in the same assembly. This does not need to be done at runtime and can easily be done with a serialized field from unity, constructing a SpellType => new Spell() dictionary in an initializer or even a lazy instantiation mechanism. In your head replace the word "reflection" with "decompiling" or "reverse engineering" to get a better sense of when its use is appropriate. Practical issue: you're hiding the relationship between Spells and SpellProcessor from your IDE. All that static analysis that you were so happy about in your Rider video goes out the window where reflection is involved. And as others have said it makes debugging a lot harder. One of the main reasons reflection is heavily used in code obfuscators.

[@JayAnAm]

@@Unity3dCollege The main reasons are: Hard to find bugs, debugging can be a pain and you can run into performance issues easily.

[@ilhameffendi2737]

this is exactly my mistake when developing a shoot 'em up game. I added a switch condition every time there's a new weapon. but it was a long time ago. the game is already on steam for 2 years now. xD

[@StephenWebb1980]

switch is not a code smell for writing editors with guilayout.toolbars where the returned int is an index representing a subeditor...I use it all the time and it's so much cleaner than a bunch of if statements

[@gigajoules6636]

Likewise in C switches are often a best possible method for doing something. Doesn't seem to translate to higher level languages in the same way though.

[@isawadelapradera6490]

Code smells are not so because of the looks of the code on the screen... I just hope noone heeds this message as advice.

[@charliemalmqvist1152]

This wasn't en example on why switch statements are bad, but an example on bad and unsafe code.

[@duramirez]

I agree 100% switch is not the issue here it's just bad algorithm.

[@AdamRoszyk]

1:56 - what plugin are you using to add informations like: "10 usages" ?

[@christopherchamberlain8477]

Not sure specifically, but Visual Studio has a thing I think called "Code Lens" and that might be a version of that.

[@OtakuDYT]

I hardly see you use scriptable objects for things, are they an issue? I tend to use them a lot for all my data entries and "spells" too since I can make great/lesser heal etc with roughly the same pattern you've listed here all in one class with a "heal" value.

[@Unity3dCollege]

Not at all, just been working on mmos (the kind w/ 10000's of spells) a lot lately and scriptableobjects don't work well in that context so I wanted to keep it even more generic :) I'm definitely gonna do some on SO's though since there seems to be a lot of interest :)

[@pawwilon]

7:50 in this case you should "default: throw..." since you don't want to ship a game with unhandled spell types. You should be careful with reflection due to it's nature to find links between different point of code more difficult since you skip referencing types directly and instead work in System.Types and genetic objects. Reflection is cool, but it can hide functionality just like switches do with it's automagical nature. Good use of it still here imo.

[@maddehaan]

This is great if you are 'convenient' in the code that is used for the dictionary the way Jason does it. I have used switch statements, but made sure (up to now, so I guess I was lucky) they would use a unique enum type (so single responsibility), like the gamemanager with a gamestate.

[@pimpace]

Hey! You say all the time when you're debugging that can't hit F10 due to it's attached to your video-recorder. Then use the brake-point command bar buttons Mate! :) (both VS and Rider have them, even they have shown in this video) You can able to use your mouse click, right? :)

[@Ziplock9000]

I'm using a version of this as a slash command processor (Like the one in EQ2) which does some string parsing first and then some parameter number and type validation. But I use Func instead of reflection.. but suppose each of the switch statements is replaced with a dictionary that points to a func or action that can have a variable number of parameters? How would you organise the definition of the Dictionary for those parameters? The way I've done it is to make the parameters a List, which works, but I'm wondering if there's something better?

[@Rexvideowow]

Trying to fit this code into my project here in 2023 and it's giving me this error: You are trying to create a MonoBehaviour using the 'new' keyword. This is not allowed. MonoBehaviours can only be added using AddComponent(). Alternatively, your script can inherit from ScriptableObject or no base class at all on this line: Spell spell = Activator.CreateInstance(spellType) as Spell; Does this only work in static classes? That's the only difference I can see between your code and mine.

[@Unity3dCollege]

It won't work with a monobehavoir because unity makes the constructor inaccessible. Should work fine in 2023 with normal classes though

[@Rexvideowow]

@@Unity3dCollege Thanks. I guess I'll see if I can avoid making it a Mono, while still figuring out a way to make an Update() tick on it, which I do need. I'm sure there is a video about that on here. Monobehaviour has gotten in my way a few times in the past. Annoying. Thanks for the answer though.

[@TheJoshy007]

Why is abstract needed after public in your spell class?

[@PhodexGames]

I never really used switches anyway because they clutter your code with breaks and I also think they are not very readable. I mean you can basically always get away with one or two liner if else statements. Much nicer to read less code. I wonder where it actually would make sense to make good use of switches.

[@MaximilienNoal]

Maybe in c#9 with pattern matching. Less code than if. Otherwise, switch was always useless in c#

[@ravanin]

Can you please take a more in depht look at statics in a video??

[@LeandroDotta]

Hi Jason! Really nice video! Do you know if the code in the Initialize method of the SpellProcessor works fine in Android using IL2CPP? I'm asking that, because I experienced weird behaviors using "Activator.CreateInstance" (with IL2CPP at Android) before. Thanks :)

[@ToadSprockett]

I've been using them on collision event handlers, they are isolated in those areas and I find them easier to read. But that's about the only area I have found them useful, it's all about context :)

[@XZYSquare]

You could use a list of spell objects, which can do the checking for whether it can be run on the enemy and can do the things each object would do. Like add health or remove health. Instead of having a "changehealth" method you could just do that stuff in an auto property. moving things out of an autoproperty into another method just to be used the same way, won't help your code. it'll still run the same.

[@smoraisbh]

Nice video. Congratulations.

[@DevVader]

Tbh I would have used a strategy pattern in conjunction with a factory pattern to achieve this. Easier to read and debug!

[@aurelienlerochbermuda6600]

yeah the factory pattern video even use spell(ability) like here, thanks for the new video

[@PerfectlyNormalBeast]

That static dictionary makes me nervous. I'm pretty sure dictionary isn't threadsafe I'm assuming all events from the unity engine are on the main thread?

[@PerfectlyNormalBeast]

If the spells aren't threadsafe, you can make the dictionary "thread safe" and do your reflection initialization by wrapping it in a ThreadLocal ThreadLocal itself is threadsafe and guarantees one instance per thread (used just like Lazy, where lazy is one instance per process) For example: private static ThreadLocal _dict = new ThreadLocal(() => InitDict()); private static Dictionary InitDict() { //TODO: load it up before returning return new Dictionary(); } Then to use it: string result = _dict.Value[3];

[@RossYlitalo]

Great Stuff! Thanks Jason!

[@alucardmikeg3]

Why do you use abstract classes even though you implements nothing inside it? Why not an interface?

[@LoOnarForge]

Hi Jason, thanks for the great lesson, as always. Quick question: those gray comments like: "Set by Unity" or "2 usages" by the functions name, is that some sort of addon for the Unity or for the VS perhaps? You are not using Visual Studio right? Thanks for the info, looks helpful.

[@brendonlantz5972]

It's a good point, but the core issue is not the switch statement - it's using the switch statement for a growing list of cases. Switch statements used with a small enumeration that is not going to grow large like (CameraMode - first person, third person) is not a real issue. The key is good judgement in how you use switch statements and abstraction.

[@Sv9zlsT]

Jason meets Rider :) and starts use "var" :) it's so misunderstunding when read others code, always tell my teammate about it... and rider sometimes doesn't work with breakpoints in static content, please remember about this underneath

[@DevVader]

What's the problem with using "var" in C#. Don't understand why people are concerned about that. "var" is just a way to safe some time. Type safety still exists with "var".

[@tryfinally]

@@DevVader var is less readable when doing code review in mspaint.

[@ConchStreetStories]

Do you have made any c# tutorial, i mean if you have masy any playlist

[@andremilanimartin3338]

now i need to figure out if there is a way to generate those spell classes at runtime from a xml file

[@bambino3624]

That sounds like a rather unorthodox approach. Also why an XML file? Isn't JSON lighter?

[@fluffyluffy884]

I have a long switch statement that is handling all my game windows showing them and hiding them -- is there a better way to handle all game windows in one UI_Manager

[@smartknowledge9209]

Thank you very much. Would you mind to do a video tutorial about level select system that when player come back next time he will start from the level he left last time(eg:- voodoo iOS game Roller splat!) without player need to go to level select screen. Just start from the level.

[@ericsilver6362]

Just save the fucking level to a text file.

[@mypaxa003]

Omg. Why I have not found this video before. I just implemented similar system digging tones of information at the internet. Damn...

[@nateHERE]

i cant see anything on the white code editor background xD

[@RasTaIARI]

Why even keep the enum instead of working directly with the System.Type Objects of the class directly? The Mapping from enum to subtype and therefore the whole SpellProcessor Class doesn't seem necessary to me ;)

[@Arkounay]

Nice trick, really interesting. Could using reflection like that lead to issues when compiling to different platforms? Since it's pretty magic I've always been trying to avoid it since I'm not sure what the limit is, but if it works well it could be really interesting for some use cases like this

[@Unity3dCollege]

It used to be an issue on mobile but since 2018 it's been pretty safe to use most .net functionality including reflection. I haven't run into a problem on any platforms lately, but if someone knows of some that it doesn't work on it'd definitely be worth noting.

[@Arkounay]

@@Unity3dCollege That's good to know. Thanks :)

[@dino339]

@@Unity3dCollege hi, when you say since 2018, you mean mobiles that are since 2017 or older can have issues?

[@Unity3dCollege]

Sorry I meant Unity version 2018.2 actually :)

[@JaviepalFordring1]

How is this design pattern called?

[@azgan123]

Why I have no idea whats going on half of the video? :(

[@caliche2000]

Your code is like a computer scientist's wet dream.

[@KhrisKruel]

Thank you, this is exactly what I've been looking for. There is almost no understandable videos on reflection. Can you also do this with interfaces or is this unique to classes?

[@RiorXD]

That spell switch issue is why i try not to pass values around when they dont need to be passed. ...

[@chesterapavliashvili1533]

Can someone tell me which text editor is used in this video?

[@Jichael38]

JetBrains Rider

[@Dasky14]

Video idea, about a problem I've had lately: Pairing saving data with ScriptableObjects. For example, I have made my game's enemies as ScriptableObjects, so that they can be made into assets easily by the designer and/or artist easily. My problem comes when I need to keep track of things like current health/mana of said character, and then I need to make some separate CombatCharacter class and... Well, It just feels like I'm repeating myself, instead of simply being able to store the current health of a character in the same place as other stats. So, in short, I'd like to see a more elegant way of pairing temporary data with ScriptableObjects.

[@matt78324]

Great video

[@Unity3dCollege]

Thanks :)

[@andurilan]

They are only a code smell for people who skim code. Of course a 15 branch switch statement is badly written code, but it would only get worse with if/elseif/else... that would be 3 times the code.

[@Unity3dCollege]

if/elseif's would definitely be way worse, no disagreement there. But I'm strongly in a mode of classes over conditionals right now and feel like most of the time the code will be easier to understand w/ more small classes and less switches/if's... (at least that's how i feel today lol, we'll see how the future is)

[@bazzboyy976]

Hey Unity3D College, great video. I have never thought about or seen this problem solved this way, I thought from this problem, you would always have to resort to passing in spell objects and throw enums away. Do you use this solution for the mmo stuff you have been working on as of late? Are there any issues as you start to have many spells with more complexity? I assume as you begin to have multiple heal abilities that differentiate from each other in some way, your spelltype enumerations would begin to take on more specific names ('holy light, regenerate, flash heal, etc)?..

[@andylinkOFFICIAL]

switches aren't code smells they are more performant than polymorphic approaches. and reflection is pretty slow so there is some overhead, unless you start caching / pooling.

[@Unity3dCollege]

Anything found via reflection should definitely be cached. And switches themselves aren't always bad, but there are a lot of scenarios where they become a maintenance nightmare. Definitely want to redo this video soon though to clarify it a bit more

[@guangzhixiao9638]

So this guy's video argument against switches is: 1) setup 2 switch control flows, one in IsValidTargetForSpellType() and another in UseSpellOnOtherCharacter(). 2) add a new switch case in UseSpellOnOtherCharacter() ".ChangeModel" 3) forget to add the same switch case handling in IsValidTargetForSpellType() 4) run the sample game and tell viewers that this is a hard to debug issue since we have added the code for ChangeModel but nothing is happening. I get that we are seeing this issue in hindsight so maybe this is a case of "hindsight is 20/20". However, this can also be avoid through the proper coding practice of adding debug print statements in control flow statements. For example: - adding debug print statements to failure cases like in default switch case handling in IsValidTargetForSpellType() - adding debug print statements under the "if (isValidTarget == false)" in UseSpellOnOtherCharacter() This way, if something unexpected happens, you can take a glance at Unity's message console and figure out the logic flow instead of just wondering "why did my change not occur". You don't have to build a strawman against switch statements just to teach reflection and claim it's code smell in general when you can use them properly. EDIT: I do agree that if you have many related functions (say casting spells in games) do their own switch statement checks it's code smell compared to creating abstract Spell class and have each derived class of Spell implement their own "isValid" and "doSpell". However, whether you use switch or long "if-else if-else" control flows, I think the key code smell is not switch statement usage but writing similar functionality (checking spell types) in many different functions. Perhaps this is just simple click-baiting and I fell for it for more views then. Ah well.

[@moofymoo]

tl;dr - hide branching with reflection, because switch are bad.. I like switch, because it is simple and clear way to shows where execution flow splits in multiple branches. If code is a mess, then problem is anything else but not a switch keyword. Problem could be bloated scope, unnecessary branching, mixed responsibilities, naming, and ..unnecessary abstractions because architecture and design patterns.

[@cuttlas2955]

so should we use Rider now ?

[@RaPL123]

Yeah, it's mandatory for all who watch this channel :-D

[@pranavkushwaha6016]

I am really a bad programmer :(. Thanks, Jason for creating these great videos and tutorials. Your videos are helping me to dig more into architecture and learn, explore, self-improvement. Thanks.

[@DenverMadsen]

Do you prefer this approach of reflection over the Gang of Four's Visitor Pattern? en.wikipedia.org/wiki/Visitor_pattern

[@AleksanderFimreite]

Thanks for teaching me about reflection. I was not aware it was something one could actually do. Up until now, for registries such as that, I have always made everything MonoBehaviours, so that I can instantiate them in some global scene, and manually add all new implementation to an serialized array in the inspector. REALLY tedious!

[@shenlong3879]

As other have already pointed out those aren't problem with switch statements, they are really problems with bad code design.

[@senser1o76]

I saw book behind you - Tupac Shakur =)

[@MalrickEQ2]

Why would you avoid switches? They are one of the most performant things in coding.

[@petrusion2827]

They're great, but sometimes you can do even better, especially with something as low level as enums and bools. The switch at 8:36 could've even been done with no branching: return (spellType == SpellType.Heal && caster.IsPlayer == target.IsPlayer) || ((spellType == SpellType.Damage || spellType == SpellType.Root) && caster.IsPlayer != target.IsPlayer) || (spellType == SpellType.ChangeModel && caster == target);

[@isawadelapradera6490]

Thing is not that switches are bad. They are possitively great specially if used adecuately. Thing is if you find yourself with a conditional tree with so many different cases, you _probably_ made a gross structuring mistake earlier on.

[@sconosciutosconosciuto2196]

Good video

[@litgamesdev]

Ah I see, the problem is not switches themselves but implementing them instead of concrete implementations of a derived type. I have a power up class in my current project I could definitely apply this to.

[@duramirez]

I would never code like that, who does that? it's not switch fault, it's your logic and algorithm that is the issue. Sometimes you try so much to separate stuff that just should not be splitted up. What i would do is put the validation inside the methods created in the target object, respecting the responsibility principle that dictates that the Class should know about itself, so each method in this class ModifyHealth(), etc, etc, should be able to tell if this particular Instance is a valid target... putting validation about a Class outside the class is bad practice (SpellTargetChecker)

[@StephenWebb1980]

So you'd rather have an object validate itself? Nah. It's a bad practice for an object to tell the rest of a system that IT, a part or product of the system, is valid. The system or another abstract part of the system sets the rules for checking if the system's objects are valid. This way you are programming, in one place, a system's validation of its parts and products otherwise you're breaking the DRY principle.

[@duramirez]

@@StephenWebb1980 What does that have to do with switches? that's architecture design, don't change the subject. But since you want to criticize on my architecture decisions, ok fine, do whatever you want, this is how i and pretty much the rest of the world do it, so keep up with your DRY. No bad practices in any of our architectures, they are just different architecture, so stop with the "nah" it's up to you how you want the system to behave, nothing is set on stone, it's not just because i chose a different approach that makes it invalid, both approaches are vetted around the world, just pick your poison.

[@DeZmanStudios]

In the "problematic case" there is nothing wrong with the switch statements. The problem you presented is the programmer not knowing how spell validation works. You can replace switches with anything but the programmer might just forget fo fix the validation function and the same will happen. Nothing wrong with having "actions" and "conditions" defined separately in source code, but the programmer needs to know about it. Your reimplementation of spells as classes with a validation method ("action" and "condition" defined in the same space in source code) is also completely fine an to me more preferable (less jumping aroud code) and is in the spirit of OOP.

[@hellangel28]

The title is very misleading, switches are NOT a code smell perse, misuse of proper structures is the issue here, switches itself are completely fine.

[@khaa5zz81]

after watching this. i just realized how terrible as a programmer i am.

[@robertmcdonell831]

You too? 🤣 I though I was the only one

[@VictorQianYT]

humm, i never thought of using Reflection to add class like that, i always add them by hand, nice trick, thanks

[@Unity3dCollege]

I used to do the same thing, but I'd occasionally forget one or someone would delete an entry accidentally in a merge, etc and I'd find myself debugging to find them. Once I found the reflection trick it simplified a lot of things and pretty much removed the possibility of me forgetting that step or breaking it there :)

[@handleneeds3charactersormore]

why would switches be so disgustingly broken in unity? or is it a C# thing?

[@BrunoWillianAndrade]

>>>CAST as Spell!!!

[@godzabomni4148]

Are your rich from comp sci?

[@darryljf7215]

I do wish there was a GitHub repo.