If I come across this code:
if (widget.Price < 10.0m && widget.UnitsInStock > 100) { // ... }
Then the first thing I'll suggest is refactoring to:
if (widget.IsEligibleForDiscount) { // ... }
"IsEligibleForDiscount will never be reused!", someone will shout. "Why do we go to the trouble if the logic is never reused?".
Ah, yes - but if we only use encapsulation as a means to generalize and achieve reuse, then we're not taking advantage of what classes, methods, functions, objects, and properties truly have to offer. As Kevin Henney once wisely said: "there is no such thing as reusable software, only software that has been reused".
"Why go to the trouble of creating a one-line method or property?".
Yes, it is is trivial, but you can't measure the effectiveness of an abstraction by it's size, or by the amount of effort required to create the abstraction. This is like measuring the impact of an animation by counting how many colors it uses. It's often the small, quick, subtle animations that improve usability and polish. Likewise, a good abstractions can be small and still improve the usability of the code.
Go away - and don't come back without hard numbers.
You've probably worked on applications that have two types of objects - objects with data and object with behavior. The objects with behavior are mostly procedural scripts to manipulate the objects with data. Strings and integers dominate the codebase. The objects aren't part of an object oriented design, but part of a process decomposition.
Breaking out of design habits is hard - it takes deliberate practice, and depends on introspective moments.
We can debate in our minds if the refactoring make the code better or worse - but let's at least have the debate.
Comments
var isEligibleForDiscount = widget.Price < 10.0m && widget.UnitsInStock > 100;
if (isEligibleForDiscount)
{
// ...
}
What if the code within the if statement was moved to a method on widget and the logic test was applied within that method? Then the top-level code becomes widget.DoSomething(). Tell, don't ask.
http://pragprog.com/articles/tell-dont-ask
We were quite surprised with the results :)
http://jsperf.com/direct-test-vs-encapsulated
private bool WidgetIsEligibleForDiscount(Widget widget)
{
return widget.Price < 10.0m && widget.UnitsInStock > 100;
}
This makes the original "if" read like this:
if (WidgetIsEligibleForDiscount(widget)
{
// ...
}
This should also be a standard refactoring on most IDEs.
Just my 5 cents :-)
IsEligibleForDiscount = (widget.Price < 10.0m && widget.UnitsInStock > 100);
if(IsEligibleForDiscount)
{
// ...
}
It totally disregards encapsulation for the sake of cohesion but it does make the code more clear.
Also to TweZz... cool testing tool, but you're refactoring needs work. The following is much faster in firefox 3.6 and slightly faster in ie8 (though the tool seemed buggy in ie8):
jsperf.com/rebuttal-direct-test-vs-encapsulated
You don't know and you don't have to know. If and when that happens you can easily refactor. It's confusing to have objects that have one-off properties like this all over the place. It's probably a violation of the Interface Segregation Principle as well.