Search Unity

Why would this method of coding be bad. Amateur question.

Discussion in 'Scripting' started by KaseyW, Oct 13, 2015.

  1. KaseyW

    KaseyW

    Joined:
    Jun 6, 2013
    Posts:
    30
    Could someone explain to me why doing this is bad? It's something I'm starting to do more and more often and I have taught myself out of convenience. But I feel dirty for some reason. And I know Static is a word i am supposed to avoid when I can.
    Code (CSharp):
    1.  
    2. public class ClassA
    3. {
    4.    public static ClassA instance;
    5.    public float valueA = 13.5f;
    6.    private void Awake()
    7.    {
    8.       instance = this;
    9.    }
    10. }
    11.  
    12. public class ClassB
    13. {
    14.    public float someValuef;
    15.    private void SomeFunction()
    16.    {
    17.       someValuef = ClassA.instance.valueA;
    18.    }
    19. }
    20.  
     
  2. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,531
    It means only one ClassA could ever exist.

    This is called a "Singleton" (well, almost, you don't enforce the Singleton... so it's not fully there). There are a lot of criticisms of the Singleton pattern because if used all willy nilly it can lead to very bad code dependencies.

    It should really only be used if/when you understand the limitations that come with.


    Furthermore, 'static' is not something to avoid. Again, it's something you need to understand what it's intended for.

    A static member of a class is no longer object specific.

    An example is the Mathf and Random class, both of these classes contain only static members, hence why you don't have to say something like:

    Code (csharp):
    1.  
    2. var math = new Mathf();
    3. var value = math.Cos(math.PI);
    4.  
    But, statics are global. And globals again can be a trap for novice designers. It again creates a situation where one and only one thing can exist.
     
    Kiwasi likes this.
  3. KristianDoyle

    KristianDoyle

    Joined:
    Feb 3, 2009
    Posts:
    63
    In one way it's a shortcut for making a reference to a class.

    In another way, it makes every reference you make later, more verbose ..
    ClassB.instance.Banana();

    instead of ..

    ClassB.Banana();

    So it's not really a shortcut after all.

    And ! It's easy to start just adding it to every class you make .. which explains the feeling of wrongness I suppose. Not assuming you do though ..
     
  4. Glurth

    Glurth

    Joined:
    Dec 29, 2014
    Posts:
    109
    I'm not sure I understand why your CLASS itself isn't static. A class defined a static means there is only ONE instance, ever- and you don't even need to instantiate it (with "new"). It can be used like so...

    1. static public class ClassA
    2. {
    3. public float valueA = 13.5f;
    4. }

    5. public class ClassB
    6. {
    7. public float someValuef;
    8. private void SomeFunction()
    9. {
    10. someValuef = ClassA.valueA;
    11. ClassA.valueA = 10f;
    12. }
    13. }

    In your example, if class A is never instantiated and Awake is never run: the value of ClassA.instance could be null when used by SomeFunction().
     
  5. angrypenguin

    angrypenguin

    Joined:
    Dec 29, 2011
    Posts:
    15,620
    Can you describe what "static" actually means? To put the question differently, can you explain why when something is static there can only be one of them? Static is subtle, tricky, and a quick path to errors and nasty spaghetti for inexperienced coders who don't fully understand it.

    To me it mostly seems "dirty" because it implies that you don't understand how to properly manage references to objects. That's cool, as everyone has to learn, and asking questions is great, so keep doing that!. If you're using that on MonoBehaviours then I'd suggest checking out the different ways to assign and find references to components in Unity. If you're using them on normal C# classes then I'd recommend picking up a book on introductory software design, or checking out the relevant chapters in your C# book of choice.

    Actually, in this case it doesn't. That looks like it's a loose implementation of a Singleton, but it isn't. There is nothing there to enforce a single instance. There's no reason that you couldn't make instances and keep references to them just like any other class.
     
  6. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,531
    Yes, true, that's why I had my parenthetical.

    But in the terms that the OP is expecting to access ClassA... only one is ever accessible.

    It may have been more specific to say "it implies only one would ever exist".
     
    angrypenguin likes this.
  7. bajeo88

    bajeo88

    Joined:
    Jul 4, 2012
    Posts:
    64
    Just wanted to give some extra thoughts here. Not going to cover the technical aspect as it has been covered enough I believe in the previous posts. Deciding if you use a static class or a singleton approach entirely depends on your style of programming I've worked in teams where half loved them and half hated them.

    The main advantage for singleton over static is the extra control it provides in terms of how and when classes get created. This may seem pointless but imagine you have thousands of classes all static and in level A you only ever use half of them. If they are static they are setup as soon as possible and so you have spent processing power and memory building all these classes for half never be used until level B. A more efficient approach while more complicated would be to initialise only the classes you need for level A. I know the amount of processing consumed depends on the constructor for the static and you could just have a manually called Init method for classes used in Level A but you have still constructed a class that you will not use.

    TLDR: My opinion
    - If the class will always be used (utils or settings etc...) make global
    - if the class is only used during a level or time specific can be static but I would usually use a singleton (if singleton just make sure you null check!).

    Hopefully you will not thing that using static or singletons are bad, they do have uses but depending on your experience you should use then carefully until your confident. :)
     
    SomeGuy22 and KaseyW like this.
  8. KaseyW

    KaseyW

    Joined:
    Jun 6, 2013
    Posts:
    30
    I don't want to pick favorites here, but your example and simple tone of speech made it super easy for me to understand! Thank you very much for that.
     
    bajeo88 likes this.
  9. hippocoder

    hippocoder

    Digital Ape

    Joined:
    Apr 11, 2010
    Posts:
    29,723
    I use the op's pattern quite a bit. I don't pretend to be the best programmer, but it's an incredibly convenient pattern for managers for which you know for sure you only have one. The one caveat is that in teams of programmers or larger teams, the possibility arises that there might exist more than one singleton, so there's that :p
     
  10. StarManta

    StarManta

    Joined:
    Oct 23, 2006
    Posts:
    8,775
    I use a slightly more advanced version of the OP's pattern. Variations on this:
    Code (csharp):
    1. public class Thing : MonoBehaviour {
    2. public static Thing main {
    3. get {
    4. if (_main == null) _main = FindObjectOfType<Thing>(); //only included if any object is likely to use Thing.main in Awake
    5. if (_main == null) {
    6. GameObject foo = new GameObject(" Thing");
    7. _main = foo.AddComponent<Thing>();
    8. //alternately, I use Resources.Load and spawn a prefab of the archetypical Thing
    9. }
    10. return _main;
    11. }
    12. }
    13. private static Thing _main;
    14. void Awake() {
    15. _main = this; //only exists so that FindObjectOfType is unlikely to actually be called
    16. }
    17.  
    The advantage of this is that Thing.main is guaranteed to never be null, so there's no need to null-check when using it.

    In OP's basic example, sure, there's basically no reason for it. However, the reasons to use a singleton can be crucial, especially in Unity. Two big ones come to mind:

    1. Ability to set and view parameters and values in the inspector.

    2. Ability to use Update and/or Coroutines. This is huge.
     
  11. Glurth

    Glurth

    Joined:
    Dec 29, 2014
    Posts:
    109
    Good points! Here is how I HAD been working around them...
    1) I create a new data-storage class derived from ScriptableObject Asset to hold the all data the static class uses. The static class can load this asset whenever it needs to, and operate off that data, (like your resources.load comment implies). Of course the user needs to know to find that asset in the editor, to change the class's values, which is indeed a bit odd.

    2) I use threads when I want my static class to START another process, though that does have serious limitations (no access to unity engine). But usually I'm calling a particular static function AS a coroutine, which has not been a problem.

    Not sure why.. but I think I got it in my head at some point that if I'm deriving from monoBehavior, the class should NOT be static or singleton, though it's fine to reference a static class or singleton, as a member, without issue. Funny how we put ourselves in these boxes without even realizing it: Thanks for making it visible, now I can get out of it..
     
  12. Korno

    Korno

    Joined:
    Oct 26, 2014
    Posts:
    518
    Singletons always are hot topics but they are sometimes valid in the right situations. Remember whenever you do EventSystem.current in your code you are using a singleton. Thats probably a valid singleton as really there should only ever be a single eventsystem in the game. XXXXSystem are usually valid singletons.

    I think the problem possibly comes from over use and the fact that they can really come back to bite you.

    Good example? Friends game was single player and used a Player.instance singleton. Later he wanted to make it single screen multiplayer and asked me to help. Was not fun. At all.
     
  13. hippocoder

    hippocoder

    Digital Ape

    Joined:
    Apr 11, 2010
    Posts:
    29,723
    I wouldn't call that a good example. It's just an example of someone who doesn't actually know what he's doing.
     
  14. ashley

    ashley

    Joined:
    Nov 5, 2011
    Posts:
    84
    I assume he meant it's a good example of why not to use it as it can come back to bite you later.

    Or "a good example of someone not knowing what they're doing". If you want to put it that way ;)