CODEWITHSUNDEEP
c#coding-styleconventions
NominSim
NominSim

Reputation: 8511

Using properties vs. methods for calculating changing values

Is there a convention for whether or not to use a property to calculate a value on call? For instance if my class contains a list of integers and I have a property Average, the average will possibly change when an integer is added/removed/modified from the list, does doing something like this:

    private int? _ave = null;
    public int Average
    {
        get
        {
            if (_ave == null )
            {
                double accum = 0;
                foreach (int i in myList)
                {
                    accum += i;
                }
                _ave = accum / myList.Count;
                return (int)_ave;
            }
            else
            {
                return (int)_ave;
            }
        }
    }

where _ave is set to null if myList is modified in a way that may change the average...

Have any conventional advantage/disadvantage over a method call to average?

I am basically just wondering what the conventions are for this, as I am creating a class that has specific properties that may only be calculated once. I like the idea of the classes that access these properties to be able to access the property vs. a method (as it seems more readable IMO, to treat something like average as a property rather than a method), but I can see where this might get convoluted, especially in making sure that _ave is set to null appropriately.

Upvotes: 2

Views: 760

Answers (3)

Eric Lippert
Eric Lippert

Reputation: 660563

The conventions are:

  • If the call is going to take significantly more time than simply reading a field and copying the value in it, make it a method. Properties should be fast.
  • If the member represents an action or an ability of the class, make it a method.
  • If the call to the getter mutates state, make it a method. Properties are invoked automatically in the debugger, and it is extremely confusing to have the debugger introducing mutations in your program as you debug it.
  • If the call is not robust in the face of being called at unusual times then make it a method. Properties need to continue to work when in used in constructors and finalizers, for example. Again, think about the debugger; if you are debugging a constructor then it should be OK for you to examine a property in the debugger even if it has not actually been initialized yet.
  • If the call can fail then make it a method. Properties should not throw exceptions.

In your specific case, it is borderline. You are performing a potentially lengthy operation the first time and then caching the result, so the amortized time is likely to be very fast even if the worst-case time is slow. You are mutating state, but again, in quite a non-destructive way. It seems like you could characterize it as a property of a set rather than an "ability" of the set. I would personally be inclined to make this a method but I would not push back very hard if you had a good reason to make it a property.

Regarding your specific implementation: I would be much more inclined to use a 64 bit integer as the accumulator rather than a 64 bit double; the double only has 53 bits of integer precision compared to the 64 bits of a long.

Upvotes: 12

STO
STO

Reputation: 10658

Microsoft's recommendation to using methods:

Use method

  • If calling has side effects
  • If it returns different values each calls
  • If it takes long time to call
  • If operation requires parameters (except indexers)

Use property if calculated value is attribute of object.

In your case I think property with implicit lazy calculation would be good choice.

Upvotes: 3

kprobst
kprobst

Reputation: 16661

Yes there is... a get accessor should not in any way modify the state of the object. The returned value could be calculated of course, and you might have a ton of code in there. But simply accessing a value should not affect the state of the containing instance at all.

In this particular case, why not calculate everything upon construction of the class instance instead? Or provide a dedicated method to force the class to do so.

Now I suppose there might be very specific situations where that sort of behavior is OK. This might be one of those. But without seeing the rest of the code (and the way it is used), it's impossible to tell.

Upvotes: 2

Related Questions