koobas.hobune.stream
Best practice for properties "get{} set{}" ?
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<Transform>())
{
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<Transform>())
{
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
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
Hey $$anonymous$$ike, 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
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.
I disagree with the first being lighter. At worst they're the same, at best, the second is lighter.
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
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 !
Yes Ray - it's the same code, just in Awake ins$$anonymous$$d 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.
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.
