Search Unity

Using properties: to be or not to be ? (advice needed)

Discussion in 'Scripting' started by glad, Nov 28, 2014.

  1. glad

    glad

    Joined:
    May 10, 2014
    Posts:
    76
    Hi!

    So, I'm a bit confused.. Here is some code:

    Code (CSharp):
    1. class SomeClass
    2. {
    3.     public SomeType field1;
    4.     public SomeType field2;
    5.     ...
    6.     public SomeType fieldN;
    7. }
    And another one:

    Code (CSharp):
    1. class SomeClass
    2. {
    3.     public SomeType field1 { get; set; }
    4.     public SomeType field2 { get; private set; }
    5.     ...
    6.     public SomeType fieldN { get; set; }
    7. }
    More flexible, isn't it ?
    However I do not like that code for two reasons:
    1. It is less readable, I think.
    2. Properties aren't serializable.
    Ok, ok.. You can tell that we could make properties to be serializable, for example in a following way:

    Code (CSharp):
    1. class SomeClass
    2. {
    3.     [serializedField]
    4.     private SomeType field1, ... fieldN;
    5.     public SomeType Field1
    6.     {
    7.           get { return field1 };
    8.           set { field1 = value};
    9.      }
    10.     public SomeType Field2
    11.     {
    12.           get { return field2 };
    13.           private set { field2 = value};
    14.      }
    15.     ...
    16. }
    So you see how simple code at the beginning became big and less readable.. However that code could be more flexible and more safe.

    So my question is: is it a good practise to use properties in Game Development with Unity ?

    Thank you in advance.
     
    Last edited: Nov 28, 2014
  2. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,537
    It's how I do it all the time.

    I come from a none-game design background in .Net and Java. It's just the way we always do it. From a long term maintenance perspective it's easier to deal with and grows with your project better.

    This can also pertain to Unity, lets say you create a public field that down the line it turns out you need to do some validation on. So you wrap that field in a property, private up that field. Well... 1 of 2 things happen.

    1)
    You keep the field name the same, which means the public property is different, which means all the referencing code needs to be updated with the new property name, rather than the private fields name.

    2)
    You change the field's name to say something with an underscore in front of it, and the property receives the original name. Now all the code doesn't have to be updated. BUT, now all your previously created GameObjects have their serialized data buggered, because that data is stored by name of the field, and the name of the field has changed.


    Now... you may consider it less readable. But I don't. I of course organize my code.

    Code (csharp):
    1.  
    2. [RequireComponent(typeof(SomeOtherComponent))]
    3. public class SomeComponent : MonoBehaviour
    4. {
    5.  
    6.     #region Fields
    7.  
    8.     [SerializeField()]
    9.     private float _speed;
    10.     [SerializeField()]
    11.     private float _bounce;
    12.  
    13.     [System.NonSerialized()]
    14.     private SomeOtherComponent _comp;
    15.  
    16.     #endregion
    17.  
    18.     #region CONSTRUCTOR
    19.  
    20.     void Awake()
    21.     {
    22.         _comp = this.GetComponent<SomeOtherComponent>();
    23.     }
    24.  
    25.     #endregion
    26.  
    27.     #region Properties
    28.  
    29.     public float Speed
    30.     {
    31.         get { return _speed; }
    32.         set { _speed = value; }
    33.     }
    34.  
    35.     public float Bounce
    36.     {
    37.         get { return _bounce; }
    38.         //no setter
    39.     }
    40.  
    41.     #endregion
    42.  
    43. }
    44.  
    Note - in most IDE's regions and properties can be collapsed to hide away or short hand all its implementation. Which also increases readability.


    Now of course. This isn't necessary. And a lot of classes that I write that are very adhoc don't get this treatment, as I just slap out the adhoc script and move on.

    But when I'm writing nice clean structured code that is intended to be extensible and reused. Say, stuff I put in my SpacepuppyFramework (open portion of it: https://code.google.com/p/spacepuppy-unity-framework/), it ALWAYS gets this treatment.
     
    Mycroft and glad like this.
  3. Patico

    Patico

    Joined:
    May 21, 2013
    Posts:
    886
    One of the basic principles of OOP says that you should hide internal state of objects. So the right way is to use private fields and public props, but all depends on your project - for pet-project it does not matters, but for big one is is impotand to cone in right way.
     
  4. glad

    glad

    Joined:
    May 10, 2014
    Posts:
    76
    Thank you for having time to answer me! Great answer! I'll look through the framework you shared.

    I am just afraid that my code will look less readable and the code size will increase. Of course it will depend on the application you write. I haven't got enough experience in Unity development so I stayed the principle "keep it simple" until I met the code of another developer who (in my opinion) write his programs may be very safe and strong but unreadable for others..
     
  5. glad

    glad

    Joined:
    May 10, 2014
    Posts:
    76
    Yes.. It depends. ) What if you do not know how big would be your project ? )
     
  6. Jessy

    Jessy

    Joined:
    Jun 7, 2007
    Posts:
    7,325
    These guidelines provide optimal ease of use in Unity regardless of access modifiers for the field or property:
    1. Start with properties instead of fields, with only the necessary get/set options.
    2. Turn a property into a field if all it would consist of is {get; set;} (think of the ; as a shortcut for {get; set;}), unless it requires serialization. PascalCase will be used for a field, until it needs to be serialized.
    3. When a field becomes serialized, treat it as a backing field for a property. Create that property at the same time as adding [SerializeField].
    4. Use camelCase for all backing fields, serialized or not.
    5. Do not use backing fields directly, outside of property blocks. The this keyword should rarely be needed, as local variables will be camelCase, and class members will be PascalCase.
    Also, don't use the private modifier when it is useless. Useless is synonymous in this case with "only useful to someone who doesn't know the default access modifiers of the language". That person won't be knowledgeable enough to make good use of the guidelines. Teach them.
    Code (CSharp):
    1. [SerializedField] SomeClass someClass1; public SomeClass SomeClass1 {
    2.     get {return someClass1;} set {someClass1 = value;}
    3. }
    4.  
    5. [SerializedField] SomeClass someClass2; public SomeClass SomeClass2 {
    6.     get {return someClass2;} private set {someClass2 = value;}
    7. }
     
    Last edited: Dec 15, 2014
    glad likes this.
  7. Patico

    Patico

    Joined:
    May 21, 2013
    Posts:
    886
    So, size of your project will depend on your codestyle )
    Using right techniques you can write a big project as well as a small, but otherwise you can create only small.

    My point - use public properties for data that should be accessible outside of class, use private fields with SerializedField attr for showing data in Inspector, and hide internal state and implementation as much as you can.

    Also don't forget about testing and think before doing )
     
    glad likes this.
  8. glad

    glad

    Joined:
    May 10, 2014
    Posts:
    76
    Thanks for the great tip!
    About the useless private modifiers: they are some kind of protection: not to give someone to modify something if there some other way (defined by developer) to do that.

    Cheers!
     
  9. glad

    glad

    Joined:
    May 10, 2014
    Posts:
    76
    Got it! Thank you very much!
     
  10. Jessy

    Jessy

    Joined:
    Jun 7, 2007
    Posts:
    7,325
    As I alluded to in my post, private is the default, so I don't believe it's ever needed for fields & properties. There may be some edge case I'm not thinking of.
     
  11. glad

    glad

    Joined:
    May 10, 2014
    Posts:
    76
    Got it. Thank you!
     
  12. glad

    glad

    Joined:
    May 10, 2014
    Posts:
    76
    Hm.. There is a difference between:
    public SomeType SomePropertyName { get; private set; }
    and
    public SomeType SomePropertyName { get; }

    With latter you can't make set even if you do that in containing class.
     
  13. Patico

    Patico

    Joined:
    May 21, 2013
    Posts:
    886
    Really? Sounds very strange. What should property return, if no one can set it? Maybe in this case it will work like a readonly fields - when we can make set only in constructor of class?
    Code (csharp):
    1. class MyClass
    2. {
    3.    public SomeType SomePropertyName { get; }
    4.  
    5.    public MyClass() {
    6.        SomePropertyName = new SomeType(); /// work
    7.    }
    8.    public void SomeFoo Method() {
    9.        SomePropertyName = new SomeType(); /// don't work
    10.    }
    11. }
    Or this property will not compile... Need to try : )[/CODE]
     
  14. LightStriker

    LightStriker

    Joined:
    Aug 3, 2013
    Posts:
    2,717
    I use properties all the time, when I need to track changes or when I "convert" it into a properly serializable format.

    I just need to add [Inspect] to my properties for them to show up on the Inspector, so it's rather easy. (See Advanced Inspector for mode detail)

    Frankly, properties are just too useful to ignore.
     
  15. Jessy

    Jessy

    Joined:
    Jun 7, 2007
    Posts:
    7,325
    Properties with private accessibility for one accessor is a case where private is not useless. private is useless if both accessors have the same accessibility.
    Code (CSharp):
    1. public SomeType SomePropertyName {get;}
    won't compile yet, but C# 6.0 allows get-only automatic properties, if they are assigned inline. I don't think you can set them in the constructor, which would make them a full replacement for readonly fields. Personally, I see no point in not just using a readonly field.
     
    Last edited: Jan 4, 2015
  16. LightStriker

    LightStriker

    Joined:
    Aug 3, 2013
    Posts:
    2,717
    Not, it's useful. Example;

    Code (CSharp):
    1.         [SerializeField]
    2.         private string assemblyQualifiedName = "";
    3.    
    4.         public Type MyType
    5.         {
    6.             get { return Type.GetType(assemblyQualifiedName); }
    7.             set { assemblyQualifiedName = value.AssemblyQualifiedName; }
    8.         }
    Unless you're talking about only implicit properties?
     
  17. Jessy

    Jessy

    Joined:
    Jun 7, 2007
    Posts:
    7,325
    I don't understand why you don't think that private is useless.
    I also don't understand that question. You might mean "automatic" by "implicit", but I still don't get it if so.

    Edit: Okay, I think you thought my "It" wasn't a pronoun. It was, so I changed it to not be, for clarity.
     
    Last edited: Jan 4, 2015
  18. LightStriker

    LightStriker

    Joined:
    Aug 3, 2013
    Posts:
    2,717
    "It" meant "Properties with private accessibility"? That's quite a stretch! :p

    Auto-implemented properties are also called "implicit", because the other way around is when you "explicitly" declare a backing field. If you google "implicit properties", it knows exactly what it means. ;)
     
  19. Jessy

    Jessy

    Joined:
    Jun 7, 2007
    Posts:
    7,325
    No, just private. That is, the keyword, not the concept of what it does. I changed the post.