.NET logic in setter and a backing field

Question

I have two properties where the latter one (CalulcatedValue) must be updated whenvever the first one (FXRate) changes.

I could have this by making CalulcatedValue ReadOnly and just calling OnPropertyChanged("CalculatedValue") from FXRate setter.

public double FXRate {
    get { return _fXRate; }
    set {
        Set(_fXRate, value, "FXRate");
        OnPropertyChanged("CaluclatedValue");
    }
}

public float CalculatedValue  
{
    get { return FXRate * SomeOtherValue;}
}

But I know that CalulcatedValue property will be called a lot (used in several LINQ queries etc). and I was considering that the getter should return the value very quickly and was planning to put a backing field for it.

public double FXRate 
{
    get { return _fXRate;}
    set {
        Set(_fXRate, value, "FXRate");
        CalculatedValue = 0; //how to set CalculatedValue since it's setter is calculating it with its logic, i.e. the value parameter is not needed ?
    }
}
private double _fXRate;

public float CalculatedValue  
{
    get { return _calculatedValue; }
    set {
        __calculatedValue = FXRate * SomeOtherValue); //consider this line takes some effort to be calculated -> instead of SomeOtherValue there might be pretty complex logic here to get the result
        OnPropertyChanged("CalculatedValue");
    }
}
private float _calculatedValue;

How should the CalulcatedValue now be set ? It doesn't need to be passed any value parameter because the logic is inside the setter. The line

CalulcatedValue=0

is where the stupid thing now happens.

Answer 1

You don't need a setter at all. You need a getter only property:

public float CalculatedValue  
{
    get { return FXRate * SomeOtherValue; }
}

If you're using C# 6:

public float CalculatedValue => FXRate * SomeOtherValue;

Edit:

Since the value calculated is time consuming, perhaps the getter only should have more "beef" to it in that you have an expanded algorithm which decides whether the property should be calculated or not.

private float calculatedValue;
public float CalculatedValue  
{
    get 
    { 
        if (ShouldRecalculate())
        {
            calculatedValue = DoHeavyCalculation();
        }
        return calculatedValue;
    }
}

Answer 2

Okay, so your example calculation is a very cheap calculation that will not yield a noticeable improvement if put into a buffered variable. For for the sake if this answer, I am assuming that the calculation you are actually doing is much more complex and expensive.

Now for properties it is always a good thing to ensure that they take the same time when executing and don't have a lot of unrelated side effects. This way your code says simple.

I think that you are already quite close to what you actually want. How ever I would avoid any unreadable hacks hidden in the setters of your property. Also I highly recommend to use a private setter here, to avoid that anyone from the outside messes with your values.

I'd go for something like this:

public double FXRate 
{
    get { return _fXRate;}
    set {
        Set(_fXRate, value, "FXRate");
        CalculatedValue = value * SomeOtherValue;
        /* The calculation is completely done here. Using the value parameter because 
         * it is local and has slightly less overhead then accessing the class variable
         * or the property we are just setting. It is in general a good idea to avoid
         * reading the property in the setter. */
    }
}
private double _fXRate;

public float CalculatedValue  
{
    get { return _calculatedValue; }
    private set {
        Set(_calculatedValue, value, "CalculatedValue");
    }
}
private float _calculatedValue;

This way you have a very cleaned up model for the properties and the calculation happens where the relevant change is actually done. At this point you also could check if the value was actually changed and skip updating your expensive calculated value in case it is not required.

Answer 3

If you need INotifyPropertyChanged notification, then add it to your FXRate property (and/or any property that might change the CalculatedValue value), and do the calculations there:

private double _fXRate;
private float _calculatedValue;

public double FXRate 
{
  get { return _fXRate;}
  set {
      Set(_fXRate, value, "FXRate");
      _calculatedValue = _fxRate * SomeOtherValue;
      // this will update any control depending on the `CalculatedValue` notification
      OnPropertyChanged("CalculatedValue");
  }
}

public float CalculatedValue  
{
  get { _calculatedValue; }
}

Or, if you want to defer the calculation to the first read (because you'll be updating your FxRate many times or something before the CalculatedValue is actually read), you could do something like:

private double _fXRate;
private float _calculatedValue;
private bool _calculatedValueIsDirty = true;

public double FXRate 
{
  get { return _fXRate;}
  set {
      Set(_fXRate, value, "FXRate");
      _calculatedValueIsDirty = true;
      OnPropertyChanged("CalculatedValue");
  }
}

public float CalculatedValue  
{
  get { 
      if(_calculatedValueIsDirty) {
        _calculatedValue = _fxRate * SomeOtherValue;
        _calculatedValueIsDirty = false;
      }
      return _calculatedValue;
  }
}

Subsequent reads of CalculatedValue will return very quickly unless the precaculated value becomes dirty again (by changing FXRate)

PS: if there's anything resembling multithreading, apply locks where necessary :-)

Answer 4

Since the CalculatedValue is the result of a calculation of other properties, it should not have a setter. If you don't want to calculate on get for performance reasons, pre calculated it like so:

public double FXRate 
{
   get { return _fXRate;}
   set 
   {
        Set(_fXRate, value, "FXRate");
        CalculateStuff();
   }
}
private double _fXRate;

public float CalculatedValue  
{
    get { return _calculatedValue; }
}

private void CalculateStuff()
{
    // This calculation is private to the class so I see no reason to not use the fields..

    _calculatedValue = _fXRate * SomeOtherValue; //consider this line takes some effort to be calculated -> instead of SomeOtherValue there might be pretty complex logic here to get the result
    OnPropertyChanged("CalculatedValue");
}

private float _calculatedValue;