Class member variables, methods and their state

Question

How should class member variables be used in combination with class methods?

Let's say I have a class 'C' with a member variable 'someData'.

1. I call C.getData(), which does not return a value but instead puts data in C.someData. The class that instantiated 'C' first calls C.getData and then uses the data by accessing the member variable C.someData.

2. I call C.getData() in the class that instantiated 'C' which is a function that returns data.

I myself prefer the second way. But it also depends on the situation and it's a small difference. Is it 'bad' to have class methods that depend on the classes internal state? What are the best conventions?

Answer 1

C.someData should be a property with public get only. You can lazy-load the value into someData when you first time call the C.someData.

public class C
{
    public C() { }

    string someData = string.Empty;
    public string SomeData
    {
        get
        {
            if (string.IsNullOrEmpty(someData))
                this.LoadSomeData();
            return someData;
        }
    }

    private void LoadSomeData()
    {
        this.someData = "Hello world";
    }
}

Answer 2

Everyone has an opinion on this, but what I've seen empirically is that state is evil. State forces you to take more information into consideration when understanding how a class behaves since its behavior is now implicitly affected by the state. Avoid it as much as your performance will allow you to. I strongly endorse the second way you mention. The first way will likely cause bugs and headaches.

Generally, some of the worst methods I've seen have signatures like void DoSomething(): it takes no parameters and returns no value. The entire point of the method lies in its side-effects, which might not be apparent if the method is poorly named.

How obvious would the first way be to someone else? Does it make sense to have something called getData that doesn't actually return any data? Does it make sense that getData manipulates the state of the object? Sometimes, yes, manipulating the object for the sake of caching is reasonable. But manipulating the object to store the value you should actually be returning is asking for trouble.

If you really feel like you must go with the first version, at least call it something more accurate like void calculateValue() or even void calculateCachedValue() which indicates that it really shouldn't be returning anything and that its entire purpose is to calculate something else.