Logic in get part of property. Good practice?

Question

When databinding my xaml to some data I often use the "get" part of a property to do some logic. Like giving to sum of totals of a list or a check if something is positive.

For example:

public List<SomeClass> ListOfSomeClass{get;set;}

public double SumOfSomeClass
{
  get
  {
    return ListOfSomeClass.Sum(s => s.Totals);
  }
}

public bool SumPositive
{
  get
  {
    if(SumOfSomeClass >= 0)
      return true;
    else
      return false;
  }
}

This way I can bind to SumPositive and SumOfSomeClass. Is this considered good practice? Even if it gets more complex than this? Or would it be better call a method and return the outcome? What about calls to another class or even a database?

Answer 1

I say yes, but try to store on a private variable de results of ListOfSomeClass.Sum(s => s.Totals). Specially if you use it more than once.

Answer 2

Having complex logic in getters/setters is not a good practice. I recommend to move complex logic to separate methods (like GetSumOfXYZ()) and use memoization in property accessors.

You can avoid complex properties by using ObjectDataProvider - it allows you to define method to pull some data.

Answer 3

Property getters are expected to be fast and idempotent (i.e. no destructive actions should be performed there). Though it's perfectly fine to iterate over an in-memory collection of objects, I wouldn't recomment doing any kind of heavy lifting in either get or set parts. And speaking of iterating, I'd still cache the result to save a few milliseconds.

Answer 4

I don't see any direct issue (unless the list is quite huge) but I would personally use the myInstance.SomeList.Sum() method if possible (.net >= 2.0).

Answer 5

For basic calculations off of fields or other properties in the collection it would be acceptable to do that inside the Get property. As everyone else said true logic should never be done in the getter.

Answer 6

Depends... if this was on a domain entity then I wouldn't be in favor having complex logic in a getter and especially not a setter. Using a method (to me) signals a consumer of the entity that an operation is being performed while a getter signals a simple retrieval.

Now if this logic was in a ViewModel, then I think the getter aspect is a little more forgivable / expected.

Answer 7

I think that there is some level of logic that is expected in Getters and Setters, otherwise you just have a kind of convoluted way to declare your members public.

Answer 8

Yes, unless it is an operation that might have performance implications. In that case you should use a method instead (as it is more intuitive to the end user that a method might be slow whereas a property will be quick)

Answer 9

Please change that getter to this:

public bool SumPositive
{
  get
  {
     return SumOfSomeClass >= 0;
  }
}

You are already using a boolean expression, no need to explicitly return true or false

Answer 10

I like your naming conventions and I agree entirely with using content such as your example in property getters, if you're delivering an API to be used with binding.

I don't agree with the point others have made about moving code into a method just because it is computationally heavy - that's not a distinction I'd ever make nor have I heard other people suggest that being in a method implies slower than a property.

I do believe that properties should be side-effect-free on the object on which they are called. It's vastly more difficult to guarantee they have no effect on the broader environment - even a relatively trivial property might pull data into memory or at least change the processor cache or vm state.

Answer 11

I would be careful about putting any logic in the Getter of a property. The more expensive it is to do, the more dangerous it is. Other developers expect a getter to return a value immediately just like getting a value from a member variable. I've seen a lot of instances where a developer uses a property on every iteration of a loop, thinking that they are just getting back a value, while the property is actually doing a lot of work. This can be a major slowdown in your code execution.