We were unable to load Disqus. If you are a moderator please see our troubleshooting guide.
Very much agree on the Law of Demeter violation. From the example, CalculateValueFrom() really looks like it wants to live on dependency1.
@Mark
In what way does storing the object increase the coupling? In both cases, SomeObject knows about Dependency1 and calls one of its methods. The difference is in when it is called, but not what is called.
There is some run-time relationship difference, since the reference to Dependency1 means it will be garbage collected sometime after SomeObject is collected. Before, it may have been collected after SomeObject's constructor ran. This isn't what I normally think of as coupling though.
@bogdan
Thinking about it a bit more, your comment shows a type of coupling between the objects none of us have called out yet. By requiring SomeObject to know that Dependency1.Calculation1() always returns the same value, the version which stores the value in a field creates an implicit coupling between SomeObject and Dependency1. SomeObject needs to know that Calculation1() is implemented in such a way that the value does not change on repeated calls.
@bogdan, @mark, @mike
I have been misled by assuming a repeated calculation will take more time than storing a calculated value in a field. Memory is not uniformly speedy. Even if a calculation always returns the same value, it can be faster to repeat a calculation thousands of times rather than fetching it from memory. It depends on caching, CLR behaviour, memory layout and so many other factors that predicting it ahead of time is quite difficult.
At the same time, predictions of some accuracy can be made by looking at the amount of code involved. If a calculation is a whole class on its own with hundreds of lines, that could make sense to save in a field. If the calculation is only a handful of operations, like adding and multiplying two numbers, it may not make sense.
I try to strive for code that only expresses the functional goal. Afterwards, if necessary, optimizations like caching calculations in fields can be made.
In the specific example Mark is discussing, SomeObject is intended to be an adapter or aggregate that exposes several information sources to a consumer. It's not intended to be an information holder for the information. And, there is already caching underneath the Dependency1 and Dependency2 classes, as they do abstract away some network calls.
There's a nice combination of the two as well. Avoid the up front work of the constructor oriented approach, especially since it's possible those values will never be needed, and the performance impact of repeating the same calculations over and over if that Property is used a lot and the value doesn't change. Calculate it on demand but cache the value in a field, use a WeakReference if memory is a concern.
Good post, thanks.
One point - if there is a calculation involved on a dependency object, fields are probably not the right vehicle anyway. Fields are perceived as "data buckets", not "workhorses" and therefore programmers frequently use fields as loop invariants (e.g. list.Count) or tokens. Invoking a calculation (that, mind you, may potentially call into a remote service, since it all depends on the dependency object's implementation) would become a major performance bottleneck.
A method would be a better alternative in this case.
@bogdan that's a good point didn't think of that.
When would you save it to the field though in the constructor?
Or would you change the method to read something like this:
public decimal Field1
{
get
{
if(calculation1 == null)
{
calculation1 = dependency1.Calculation1();
}}
return calculation1;
}
}
It definitely depends on what "Calculation1" actually does. If it always returns the same result it makes sense to save it in a field instead of doing the calculation thousands of times during the life of the app.
@Dave Didn't see your second reply until today! Coupling wise I was thinking that if you stored it that meant a greater coupling but I think you're right it knows about the Dependency so there's no difference.