On one hand I see the data dependency and information redundancy as a possible issue. However on the other hand performance will suffer if the value of a costly calculation is not saved and has to be calculated with each access.
There is no one-size-fits-all answer here. It depends on the cost of the calculation, the likelihood of needing the value, how often you're going to need the value, and how much you're trying to squeeze for performance (pro-caching) vs memory footprint (pro-recalculating).
Furthermore, the (im)mutability of the class very much influences how easy it is to figure out if any changes were made to the base data (which the calculated value relies on) and therefore if the value needs to be recalculated. Figuring that out can be a massive headache and is very easily avoided by simply always calculating the value without caching it.
There are plenty of cases where you don't need such a performance boost, and then I'd rather skip the code complexity and source for potential bugs, in favor of keeping things readable even if negligibly less performant.
If you are squeezing for performance though, lazy loading is always an option. The general idea here is to not pre-emptively calculate the value, only calculate it when requested, and from that point on remember the calculated value.
In C#, this can be done manually using properties:
public class MyClass
{
private int? myValue;
private int MyValue
{
get
{
if(myValue == null)
myValue = VeryExpensiveCalculation();
return myValue;
}
}
private int VeryExpensiveCalculation()
{
return 1 + 1;
}
}
This gets us the best of all behaviors:
- If we never access
MyValue
, VeryExpensiveCalculation
will never be run.
- When we access
MyValue
, VeryExpensiveCalculation
will be run.
- When we access
MyValue
again, VeryExpensiveCalculation
will not be run again, since we already did so before.
However, this approach has a few weak points:
- You need a default value to indicate that the calculation has not been run yet. I used
null
to that end, but this might not work in your use case (when null
is a meaningful value). This can be worked around using an extra boolean parameter, but it further increases code complexity and the potential for things to go wrong.
- You're relying on the cooperation between a private field which stored the calculated value, and a public property which triggers the calculation of that value. But when you are dealing with logic inside the same class, you cannot hide the private field from sight. This makes it possible for developers to mistakenly use
myValue
instead of MyValue
or even access VeryExpensiveCalculation
directly, which bypasses all of the work we just put into this.
.NET has a Lazy<T>
class which has made the above approach more reusable and easier to implement. This also helps cover some of the weak points.
public class MyClass
{
public Lazy<int> MyLazyValue { get; private set; }
public MyClass()
{
this.MyLazyValue = new Lazy<int>( () =>
{
return VeryExpensiveCalculation();
}
}
private int VeryExpensiveCalculation()
{
return 1 + 1;
}
}
This works the same way. VeryExpensiveCalculation
will only be executed when you access MyLazyValue.Value
, and it won't be executed when we access MyLazyValue.Value
again.
There is still a bit of a technical cost though. You now have to access an additional nested property (Value
). This can be worked around using custom properties, but then we run into the same weak point as before where private logic can bypass the custom property, on top of creating even more code for our implementation.
We also still suffer from the same problem where figuring out to perform a recalculation is not obvious.
To summarize, you can cache the calculated value to save on calculation performance, but be aware that this comes at the cost of increased code complexity and the possibility of bugs with knowing when (not) to recalculate the value, which means you should only implement this kind of caching when the benefit outweighs the cost in your particular situation.