"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!
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. 😄
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 <a href="#" class="seekto" data-time="1045">17:25</a> - "we cast it as a spell" lol. nice U can create a static constructor instead of having an Initialize method.
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).
You create a new type for this combined behaviour and implement multiple health changes within method that actually implements damage in the concrete class
@@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.
@@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 I don't think there are. People who know state-of-the-art MMO architecture usually don't make RU-vid content, they are too busy working on their MMO projects. In addition RU-vid 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.
I think using Scriptable Object classes for these cases are event better than this approach. You can use the name of the object as key and have what you wanna do inside the class. This goes for states as well. Whenever i use enums the actual solution is almost always scriptable objects.
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.
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 !
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.
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.
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!
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¿
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.
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.
@@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.
"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.
More of this please! I’m self taught programmer so I don’t know any of the best practices. @<a href="#" class="seekto" data-time="114">1:54</a> 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!
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!
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.
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.
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!
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?
@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.
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?
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
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!
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.
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 :)
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.
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!!
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.
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 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.
@@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.
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.
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.
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
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.
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.
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.
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)
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 :)
<a href="#" class="seekto" data-time="470">7:50</a> 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.
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.
This was very helpful thank you. I'm curious how you would tackle interactions between a player character and an interactable object, it's an area I always end up using switches and later regretting it. My solution in the past was to have an Interactable base class that each interactable inherits from, rather than an InteractableType enum, but then you get into the interactions themselves and you go another layer deeper into the abyss
I'd definitely go with the interactable base or an IInteractable interface. I was thinking about this earlier today and I think "classes over conditionals" succinctly describes my preference currently. Make more small classes and far fewer branching conditionals, it makes things so much easier to follow once you know the base architecture of the project.
@@Unity3dCollege yeah I completely agree, it makes for better testability too, and can push you more towards a functional programming or composition way of structuring your logic. Speaking of which, do you have any plans for a DOTS course or series? It's been so difficult to study because it keeps changing, seems like it's getting closer to its final form now though
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)
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.
@@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.
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.
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.
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
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? :)
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?
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 ;)
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?
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?
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.
The biggest difference is the # of places you have to touch/modify to make a change or addition to the game. With switches it usually starts off being 1/2 places but in a bigger project you can quickly find it being a dozen or more spots that you have to add new cases to every time you add something or modify whenever you change functionality. When they're split into separate classes and encapsulated away, it leads to the functionality being in 1 place making it a lot safer and easier to modify/understand.
@@Unity3dCollege in this specific example say I have a switch what spell the unit should do. I have entities with a variable int. Every turn each entity runs a code and have a switch for their action and reads that int. On the cases they call a fuction with the name of the spell. As I keep adding spells I only have to have the units with new ints, the switch case for those ints and they call the new fuction of the new spell. For your approach the only thing I save is putting a new case on the switch. Because by adding the spell to the unit, when running the game if the turn/AI picks that spell it automatically calls it (skipping having to put the case in the switch). Correct?
@@RyuDouro I think for your case, you're in a situation where you haven't yet found a need for more switch statements. The video is targeted at projects that have found more than one place to use a switch statement for a type. You could still find yourself in need of expanding your spell system later (for example: verifying the target of your spells, like the video suggests), but changing your code now doesn't seem necessary until that happens.
@@Raymoclaus I see but I'm trying to see if this could be useful for one of my projects. Right now the spell (function) checks if the entity still exists before applying the damage to it. This is because I have multiple spell types that have different multiple targets, aoe and also friendly target or multiple allies. Plus those targets must also be stored in another list because I need to keep track of who kills what, total damage and healing and also condition and healing over time even if the owner of the spell is already destroyed.
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);
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.
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
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".
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.
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!
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 :)
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.
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.
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 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.
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.
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 :)
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 :)
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.
Well he has good points. He is just not showing the real deal cause he has no time for that in a RU-vid video. Nobody is telling you to use if else's they are in the same category of switch statements. Use "Rule of three". Meaning; If you had written or copied similar code 2 times and now you are doing it for the third, ask yourself if this can be substituted. And those "proper coding practices" with switches won't help you with writing unnecessary code over and over again. Also reflection is irrelevant here. What he is focused on is using reflection to solve a problem. And this pattern is pretty well known, it's called "Factory"
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)?..
The only reason you are saying switch is bad is because of the bad design presented here. You could very well have just one single big switch take care of everything instead of breaking it down into other methods / classes and the spell itself could take care of the isvalidtarget and so on .... I really see no reason to give swtich a bad rep because of this example. You complain about switch and then go ahead and implement a singleton to take care of spells.... go figure...
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
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.
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?
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];
