Thursday 10 March 2016

Prefer getters over directly accessing private variables

While doing some regular debugging, I found a rather interesting property of how variables are evaluated during load time in C#. Consider a scenario as follows.

Let us say we have a class that is connecting an application to another environment. So, instantiating this class can fail in the absence of an environment. We shall simulate that by just throwing an exception for now.

    public class Class1
    {
        public static Class1 GetInstance(){
            throw new Exception();
        }

        public void Functionality()
        {
            // some (hopefully) usable functionality
        }

        private static Class1 singletonInstance = new Class1();
    }

Let us consider the following class using  the above class.

    public class UserClass
    {
        private static Class1 variable = Class1.GetInstance();

        public static void SomeFunctionality()
        {
            variable.Functionality();
        }

        public static void SomeOtherFunctionality()
        {

        }
    }

Let us write a test for the above class.

    [TestClass]
    public class UnitTest1
    {
        [TestMethod]
        public void TestMethod1()
        {
            UserClass.SomeOtherFunctionality();
        }
    }

Now this test fails even though the code path being tested does not involve the exception being thrown. That is because when UserClass  is loaded, its private member's instantiation fails due to the exception.

If we modify UserClass as follows, the test passes fine.

     public class UserClass
    {
        public static void SomeFunctionality()
        {
            Class1.GetInstance().Functionality();
        }

        public static void SomeOtherFunctionality()
        {

        }
    }

The reason is simple: the static call to GetInstance() within the static method is not evaluated because it is not in the code path being executed. In retrospect this is not very un-intuitive.

However it gives me a reason for preferring getters even within the class when lazy evaluation is desired.

        public static void SomeFunctionality()
        {
            getClass1().Functionality();
        }

        private static Class1 getClass1()
        {
            return Class1.GetInstance();
        }

        public static void SomeOtherFunctionality()
        {

        } 

2 comments:

Kate said...

I don't understand one thing. In the initial scenario, while getting instance of user class, it is using some other class which throws exception. No while running units tests, why didn't you mock that external facing calls?

what you have justified is the use of static variable not setter/getters.

Phoenix said...

Use of static variable is causing the trouble in the case presented.