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

[ronronmx]

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

[anon85704231]

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

[Ray-Pendergraph]

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.