Best practice for properties "get{} set{}" ?

Question

I recently started using properties to encapsulate my private variables, and I have a question about best practices when encapsulating a private variable.

Right now I have the following, which works fine:

public GameObject playerGO;

 private Transform bikeCollidersParent;
 public Transform BikeColParent
 {
     get {

         if (bikeCollidersParent == null)
         {
             bikeCollidersParent = new GameObject("BikeColliders").transform;
             bikeCollidersParent.parent = playerGO.transform;

             if (bikeCollidersParent == null)
                 print("No Colliders Found in player object");
         }
         return this.bikeCollidersParent;
     }
     set {

         bikeCollidersParent = value;
     }
 }

 void Awake () {

     // Cache player gameObject
     playerGO = GameObject.FindWithTag("Bike");


     // Cache player's transform components
     foreach (Transform t in playerGO.GetComponentsInChildren&lt;Transform&gt;())
     {
         if (t.name == "Collider")
         {
             bikeCollidersParent = t;
         }
  }

As you can see, I set the value of "bikeCollidersParent" in Awake(), and if nothing is found in Awake(), I create a new GameObject within the "get{}" accessor in the property.

Now I'd like to keep everything related to the "bikeCollidersParent" in the "get{}" accessor, so I can do that with the following code:

public GameObject playerGO;

     private Transform bikeCollidersParent;
     public Transform BikeColParent
     {
         get {

             if (bikeCollidersParent == null)
             {
                 // Cache player's transform components
                 foreach (Transform t in playerGO.GetComponentsInChildren&lt;Transform&gt;())
                 {
                     if (t.name == "Collider")
                     {
                         bikeCollidersParent = t;
                     }

                     if (bikeCollidersParent == null)
                     {
                         bikeCollidersParent = new GameObject("BikeColliders").transform;
                         bikeCollidersParent.parent = playerGO.transform;

                         if (bikeCollidersParent == null)
                             print("No Colliders Found in player object");
                     }
                 } 
             }
             return this.bikeCollidersParent;
         }
         set {

             bikeCollidersParent = value;
         }
     }

     void Awake () {

         // Cache player gameObject
         playerGO = GameObject.FindWithTag("Bike");

      }

Which of these 2 blocks of code is the most appropriate version? What's the difference between putting the "foreach()" loop in "Awake()" and putting it within the "get{}" accessor?

Thanks in advance for any help!

Stephane

Answer 1

Best Answer

Answer by Mike 3 · Apr 01, 2011 at 05:59 PM

The difference is in that you'll only need to calculate the parent if you try access it, which is usually a good thing

The one potential downside is that doing it in awake hides the computation with loading, so you may notice it less there, but i doubt you're going to notice that

Add comment · Show 2 · Share

ronronmx · Apr 01, 2011 at 09:32 PM 0

Share

Hey Mike, thanks for the answer. When tou say "you'll only need to calculate the parent if you try access it", you're talking about my second implementation, correct? If i understand correctly, putting the "foreach" loop in Awake() automatically calculates the parent, but if I put it in the "get{}" accessor, it doesn't get calculated until I try to access it, for example, from another script's Awake() or Start(), is this correct?

Thanks again

Mike 3 · Apr 01, 2011 at 09:50 PM 0

Share

Exactly right, yes.

Answer 2

Best Answer

Answer by Ray-Pendergraph · Apr 01, 2011 at 06:02 PM

Well, to start.... kudos on choosing using C# and using properties in your design. I tend to favor the lighter (the first) implementation. There are no hard and fast rules on this and I find the right side of my brain second guessing the left side constantly about property 'weightiness'. The .NET site has a lot to say about things such as this (and a lot of other best practices stuff), but this in particular:

It is a bad programming style to change the state of the object by using the get accessor. For example, the following accessor produces the side effect of changing the state of the object each time the number field is accessed.

I disagree with the strictest interpretation of the first sentence, but agree with the statement as a whole. I tend to favor lazy initialization (createt em' the first time they are requested) which disobeys this dictum and is exactly what you have done here. To me it just takes some thought to determine the right amount of grease for the job. It usually boils down to unintended consequences and usage of the property. If the code for the getter lazily creates the resources that are required to fulfill the property correctly, I think all is good. If there are other strange state changes in the getter, that's bad. The second implementation might fall into that, but neither look obviously wrong to me.

Great question.

Add comment · Show 5 · Share

Mike 3 · Apr 01, 2011 at 06:08 PM 0

Share

I disagree with the first being lighter. At worst they're the same, at best, the second is lighter.

Ray-Pendergraph · Apr 01, 2011 at 06:45 PM 0

Share

Really? So a foreach loop and additional state change in that loop is lighter than none of that? http://en.wikipedia.org/wiki/Cyclomatic_complexity

ronronmx · Apr 01, 2011 at 09:34 PM 0

Share

Ray, thanks a lot for the explanation...I will read more about it on the .NET site. I don't know much about C# properties yet, so any help is greatly appreciated !

Mike 3 · Apr 01, 2011 at 09:51 PM 0

Share

Yes Ray - it's the same code, just in Awake instead of the get, which means it'll be calculated regardless of you using it. The get one calculates it once, and only if it's accessed.

Ray-Pendergraph · Apr 02, 2011 at 12:26 AM 0

Share

I see what you mean, what I meant was that the code in the property was more complex,not the entire solution. I thought the post was about the best properties of writing properties.... my bad.

Best practice for properties "get{} set{}" ?

2 Replies

Your answer

Welcome to Unity Answers

Follow this Question

Related Questions