Recently at my company we’ve had a bit of a debate about abstraction vs. simplicity. One school of thought I’d characterize as “DRY and abstraction can do no harm,” and leads to code like this:
def make_foo_binary(binaryName, objFiles, fooLibsToLinkAgainst)
make_exe_task(binaryName, objFiles.ext('.o'), fooLibsToLinkAgainst)
end
and this:
class String
def escape_space
return self.gsub(' ', ' ')
end
end
My point of view is that creating an abstraction like this, that’s only used in one place, makes the code less readable, since you’re replacing a function call that the reader is familiar with (gsub) with another that they’ve never seen before (escape_space), which they’ll need to read if they want to understand how the code actually works. The replacement is essentially described in English (“escape space”) and english is notoriously vague. For example, without looking at the definition, you don’t know whether it escapes all whitespace, or just the space character.
There’s lots of writing that sings the praises of DRY and abstraction. Is anybody aware of sources that describe abstraction’s limits? That sing the praises and discuss the pragmatics of keeping code simple?
Edit: I can find texts that encourage simplicity in life or in (English) writing, e.g. Thoreau’s “Simplify, Simplify!” or Strunk and White’s “Vigorous writing is concise.” Where’s the equivalent for programming?
3
Sure: Joel Spolsky (you may have heard of him) says:
All non-trivial abstractions, to some degree, are leaky.
Abstractions fail. Sometimes a little, sometimes a lot. There’s leakage. Things go
wrong. It happens all over the place when you have abstractions.
Also, see KISS and YAGNI which Jeff Atwood discusses:
As developers, I think we also tend to be far too optimistic in
assessing the generality of our own solutions, and thus we end up
building elaborate [solutions] around things that may not justify
that level of complexity. To combat this urge, I suggest following the
YAGNI (You Aren’t Gonna Need It) doctrine. Build what you need as you
need it, aggressively refactoring as you go along; don’t spend a lot
of time planning for grandiose, unknown future scenarios. Good
software can evolve into what it will ultimately become.
(see the link for the exact quote)
My interpretation of these quotations:
-
It’s really hard to create good abstractions — it’s a whole lot easier to create crappy ones.
-
If you add an abstraction for which there is no need, or which is wrong, now you have extra code that needs to be tested, debugged, and maintained. And if you have to go back and refactor, you now have more dead weight dragging you down.
1
I am not familiar with the language or runtime of your examples, but gsub does not mean anything to me. However, escape_space is a lot more meaningful. I completely disagree with your argument that familiar function calls should not be replaced by calls to functions they’ve never seen before. If I were to take this argument to the extremes, then one should never add a new function to a program: it always abstracts familiar function calls away and always replaces them with this new unknown user-defined call.
A developer should never ever have to read the code to understand how it works. First of all, he or she should not have to care about how it works, but should be able to understand how to use it and what effects it has. If this is not the case, then there a problem with the name and signature of the function, or its documentation.
To me, the goal of abstracting is to remove inner details and provide a cleaner interface to some functionality. In the future, the inner workings of the escape_space function might be changed, or overridden. I don’t care that some function gsub is called with two arguments. I do care that my spaces are escaped. So that makes the abstraction useful.
In my favorite language, C#, I always add documentation to all my functions (even private ones) describing things like the function, use, used units, thrown exceptions, input contract and types. Then, with Visual Studio’s IntelliSense any developer can see more clearly what the function does without reading the code. Without a tool like IntelliSense, developers will have to be more specific in their function names, or keep additional documentation somewhere easily accessible.
I don’t think abstractions should ever need to be limited, and I don’t know of such a source.
4
When talking about abstractions, I think it’s important to define two terms: Inherent and incidental complexity. Inherent complexity is the complexity of the problem that the abstraction is solving, incidental complexity is complexity that the abstraction is hiding.
Good abstractions are ones that hide lots of incidental complexity and try to solve the right problems, lowing their inherent complexity. I think what you’re getting frustrated with here are abstractions that are too shallow; they don’t hide much incidental complexity and it is as difficult (if not more so) to understand the abstractions as it is to understand their implementations.
My answer is going to go more off of personal experience, but a quick search found this blog, which looks like it goes into pretty great detail about when abstraction goes too far (not just the linked entry, but there are several related ones).
Basically, abstraction can be a good thing. However, like anything else, one can get carried away with it. Your examples are good ones where abstraction has gone too far. You’re creating “pass-throughs” that basically do nothing but rename the function in question.
Now, in the case of custom functions, this might be necessary, if you’re looking to deprecate an old naming scheme in favor of a new one (such as changing escape_space to escapeSpace or escWhitespace, or whatever). For library functions, though? Overkill. Not to mention counterproductive. What have you gained from making that abstraction? If that’s all it’s doing, then all you’ve gained is the headache of having to remember that function and pass that knowledge to someone else (whereas, with library functions, developers are either already aware of them, or should be able to type ruby gsub into Google and find out what it means).
I recommend being more intelligent about determining when to abstract something out. For example, things that are more than 2-3 lines, and used at least twice (even if there are slight differences, a lot of times, those differences can be parameterized), are generally good abstraction candidates. Sometimes, I’ll consider large blocks of code that do a particular thing within a larger function, for the sake of readability and the “one job” principle, and abstract them out in to protected functions within the same class (often in the immediate vicinity of the function that uses it).
Also, don’t forget YAGNI (you aren’t gonna need it) and premature optimization. Is this currently being used more than once? No? Then don’t worry about abstracting it out right now (unless doing so creates a substantial readability gain). “But we might need it later!” Then abstract it out when you actually need it somewhere else. Until then, you just end up with pointers to pointers to pointers for no real benefit.
On the other side of this coin, when you do abstract something out, do so as soon as the need becomes evident. It’s easier to move and tweak two or three blocks of code than 15. Generally speaking, as soon as I find myself copying blocks of code from somewhere else, or writing something that looks very familiar, I start thinking about how to abstract it out.
2
First, have a look at this example by Robert C. Martin:
http://blog.objectmentor.com/articles/2009/09/11/one-thing-extract-till-you-drop
The reason for this kind of refactoring is consequently building abstractions to create very small functions, so that every function does just one thing. That leads to functions like the ones you showed us above. I believe this a good thing – it helps you to create code where each called function within another function is on the same level of abstraction.
If you think using names like make_foo_binary or escape_space make the code less readable, than as a consequence, you should try choose better names (for example: escape_space_chars_with_backslash) instead of abandoning the abstraction.