Refactoring wars and how to avoid them: what is "simplicity" in programming?
reprog.wordpress.com
reprog.wordpress.com
On the evidence I have found to date, I am coming around to the view that the style of writing many very short functions (say up to 5-6 lines) with little complexity in the logic (say just a single level of nesting) is one approach whose claimed superiority is directly contradicted by empirical data. For example, McConnell discussed the number-of-lines issue in Code Complete years ago, citing multiple studies. Anyone can find still more by investing a few minutes in Google Scholar searches.
Alas, that does not stop bloggers, consultants, trainers and book authors from advocating this programming style, even though it invariably results in the kind of incohesive "spread" that the article mentioned.
(It may seem like the Java programmers simply use more capable tools, but most of them don't use incremental search at all, so they're much slower at finding something that they don't have a hop-to-X button for. Why? Because they get by fine without it, just like the C++ programmers get by without learning whatever emacs or vi extension would help them navigate Java-style source directory hierarchies.)
Obviously some kinds of tools and programmers will be more common than others, so one style can be empirically better than another yet be worse for a particular group of programmers. There won't be one better way of writing code until everybody has the same expectations and the same tools. I don't think we ever want to reach that kind of consensus.
The question becomes: when does hiding complexity improve your ability to reason about the code?
I agree with the author that this is ultimately a personal preference. One guideline that I appreciate is that your code should operate at a consistent level of abstraction. For instance, in the routine that defines the business logic, the details of the protocol used for persistence should be "somewhere else." This complicates tracing the execution of the program, but it makes understanding the intent of the business logic easier as there are less "implementation details" to ignore.
When you trust the abstraction. For example, on the whole we trust the filesystem abstraction, we rarley try to inspect the phisical location of the bits on the disk - the filename is sufficient, we can treat the fielsystem as a black box.
On the other hand, if you don't trust the code, you're going to want to read every line, and that's easier to do if all the code is in one method. An abstraction is a liability if you can't trust it.
Re testing: it's easier to trust a well tested function, so splitting up code to make it easier to test, and then testing it, will allow you to ignore it's implementation. Suddenly it isn't logic spread across six classes, but one class using some utility methods.
In my experience, the ultimate in abstraction is anything that works solely with immutable data. This includes, obviously, pure functions, but also objects which have no mutable state. Such things are trivial to test and perfectly composable. It's easier to gain trust in them as you rarely need to know exactly what they do, only whether they appear to work or not.
Obviously immutability alone is not enough for many abstractions, but I would still like to see it used more. The current object-oriented models seem to encourage encapsulating state, rather than encapsulating data, which seems to lead programmers to use mutable things even when they are unnecessary.
Unless the abstraction requires it (eg. IO), I think any state held within an object is, in a way, leaky abstraction. As long as there is state within an object, it is not obvious whether you can, at any one time:
1) call its methods 2) hold a reference to it 3) delete it, or your reference to it (Mostly applies to non-GC languages) 4) Pass it as a parameter to something
All these worries disappear with immutable values.
What's needed is pure functions for logic, and dumb objects holding data, and function pointers.
I agree. I would also say that in cases where the abstraction doesn't make sense you have to know what's going on inside the box in order to trust it.
In your example, the file system as an abstraction makes sense because once you understand that it's a tree, you are pretty much good to go.
I remember working on some projects early in my career with a very senior developer who had a habit of making the strangest abstractions possible on a problem. For example, need to use a class to define a triangle? His constructor might take 7 arguments! The center of the triangle on the screen, and the distance from the center as well as the degrees of rotation from the center (with the bottom of the screen being 0 degrees). Oh, and actually, it didn't take 7 arguments, it took 4, but three were some special object type containing a tuple of radians plus a mishmash of other parameters and a coordinate on screen. Because he wanted to, you know, reuse that code again. So in order to build a triangle, I had to define a center point on screen (with 0,0 in the middle of course because that's how his code worked) and then calculate the coordinates of the three triangle points from the distance I wanted, then convert degrees into radians or some such....bah! I don't remember all of the details, this was a long time ago, but it was horribly obfuscated and made no intuitive sense. He explained that it was all to avoid some singular edge case that he had encountered once, and he thought it was a good trade-off because all of the new edge-cases it introduced were manageable.
When I received this code (without documentation or useful comments), he was on vacation for two weeks.
Naively, I assumed it was an easily understandable abstraction in that I could simply supply 3 coordinates in some order to the library and get a triangle. I spent a couple days trying to figure out the order I was supposed to issue the coordinates to get it to draw before giving up and just reading the code to figure it out. Worse yet, the internals were abstracted all to hell in a similar obfuscated fashion and I literally got nowhere in trying to figure it out.
I actually just waited for him to get back to walk me through the code, peppering him with question like "do we really need to define the z coordinate as 0 all the time since the display is always 2d?" before proceeding on that work. Once I understood it, I just wrote some wrapper code to translate three normal 2d coordinates into his craziness to simplify my life.
I ran into this kind of thing with his abstractions all the time. From the most insane string class you have ever seen to a home rolled virtual memory library that pickled objects onto disk, but all of your objects had to be built around a base class that was full of useless virtual functions that you had to implement.
Recently at my day job, I came across a string trim() function, which started with this comment:
// removes leading and trailing whitespace. Also removes commas.
The code itself also removed single trailing periods. That really hurts your trust in the code base - you need to read everything a function calls to understand it, you can't trust the method names. This makes it take longer to understand any particular piece of code - even if in the end the methods called actually do what they claim to.I don't think it can be drilled into developer's heads enough, that someday, somebody else will have to deal with this code. And that person might even be them 4 years from now.
How about this: "when the degree of complexity hidden is greater than the extra complexity introduced by the abstraction mechanism"?
Edit: Here's an example to show what I mean. Suppose we have a 2D array representing a matrix of values and we want to print it. We could write an algorithm like this:
function print_matrix =
for each row i
for each column j
print_element i j
Or we could write something like this: function print_matrix =
for each row i
print_row i
function print_row i =
for each column j
print_element i j
In the second version, we might have shorter functions and less nesting, but we haven't significantly lowered the level of abstraction between one function and the other, so there is little real benefit. On the other hand, we have reduced the cohesion because now the reader must follow the logic through two functions instead of one, and this is bad. There is too much extra complexity created by introducing the second function and not enough hidden complexity because the levels of abstraction aren't much different, so breaking out the inner loop is a bad trade-off that makes it harder to reason about the code.Granted, I don't like spreading related logic over several files, but that is Java's fault.
Haskell functions aren't shorter because someone decided one day that they should be, they are shorter because Haskell is relatively expressive in some areas. Concepts in suitable areas that would take several lines to represent in another language might take only a line or two in Haskell.
On the other hand, there are other areas where Haskell takes pretty much the same amount of lines as an imperative language, and still others where Haskell's model isn't such a good match and it might require more lines.
I think you have it exactly right, and I wish I'd made that point, as clearly as this, in the original article (or indeed in the followup, which I stupidly did before reading the HN comments about the original.)
A function/method is like a paragraph in prose writing. Any good style book will tell you that there is no "right length" for paragraphs (although bad teachers might teach rules like "no less than three lines, no more than ten". A paragraph should be exactly long enough to convey one clear point, whether that takes one line, ten, or twenty. The same for functions.
By the way, I'd like to say that a LOT of the comments on this thread are really insightful (i.e. the include insights that are new to me, but which immediately make sense once I see them written down). It's pretty humbling to see this community so quickly come to so many valuable conclusions when my poor, bumbling article took so long to get to where it did. Thanks to all who have contributes -- I hope you'll stick around on The Reinvigorated Programmer and contribute to the discussions on there, too.
And with testability comes flexibility and maintainability. I think the author is focusing too much the simplicity of familiarization and conceptual weight where, very often, the real challenge with imperative code lies in maintenance.
But the more important reasons to strive for high cohesion/low coupling are: Future changes are generally easier when you have well defined blocks instead of a birds nest of code, unit testing practically writes itself, and you have a better chance at isolating a problem to a module if it's responsibilities are few and well defined.
Simplicity is having to think about a smaller number of items and dependencies when I want to change/extend/reuse code. If there are many functions and classes but I have to touch all of them whenever I want to make a typical modification, then it's worse than having everything in one function. Conversely, if a more granular design allows me to ignore most of the code and just change one simple function or even just parameterise it differently, that's simplicity.
But there is a snag. Even if the design aligns well with units of change and reuse, and I would have to make just one small change in one small function in order to have the desired effect, I don't necessarily know which of a large number of functions it is. I might not even know whether or not such a function exists. So I have to understand how everything works together in order to benefit from well designed code.
That leads me to the conclusion that better programmers who do understand the system and its interdependencies benefit from small units provided they align well with units of change and reuse. Bad programmers have to look at everything every time anyway, so they might find it easier to look at one large chunk of code.
It makes total sense to split things into 7 different classes when you actually need different implementations of all those parts so that the abstractions you've made are useful.
The problem with demonstrating these things in books (or in general) is that your examples have to be simple to be comprehensible. But the presumption is that the techniques are being applied into reality to a much more complex system. I'm sure if the book had injected 20,000 lines of code into the example he would have written a blog post about how the book should have used a simpler example to demonstrate the point while having no complaint about the fact that it used 7 classes to do it.
I think because of this tendency in books and courses to demonstrate complex OO techniques with simple examples many people come away with the attitude that you should do all this stuff pre-emptively rather than "on demand". I'm not sure that was ever really the intention, but it has resulted in a lot more overly abstracted code being produced in the world than necessary.
But look at the cost: to understand how rentals are calculated, we now have to read six classes instead of one method... Fowler evidently finds it easier to read many small methods than a few larger ones; I find the opposite.
This raises the question, do we all have our own definition of good code?
Several books have tried to formalize what is good code (he mentions Refactoring, Code Complete also comes to mind). I enjoyed those books, but I'm often reminded of what my CS professor once told me: good code is a matter of taste.
In the example he cites from Refactoring, my taste is more like Fowler's. I think it's easier for bugs to hide in long methods than short ones. Plus, Fowler extracted some distinct concepts into their own classes-- things like prices. Price formulas are likely to change, so I say the cost of extracting that class is well worth it.
It seems to me that ultimately, Fowler and others are not claiming to have found the secret to "good code;" rather, they are trying to influence people's taste for what they consider good code.
Massive functions start out as small and then large ones. If a business logic function has to grow every time there's a new rule that could live behind an abstraction, it's on the path to becoming massive.
The happy middle is "just right", but it's hard to recognize or achieve without some experience of either extreme. And even then, the chosen point might be biased towards less abstraction for performance reasons or more abstraction for composability and reuse reasons. But really good use of abstractions (ideally including at the language level) ought to reduce the amount of compromise needed.
1: From the Agile Manifesto (http://agilemanifesto.org/principles.html):
Simplicity--the art of maximizing the amount of work not done--is essential.
2: From Kent Beck on Extreme Programming:
Simplicity is the most intensely intellectual of the XP values. To make a system simple enough to gracefully solve only today's problem is hard work. Yesterday's simple solution may be fine today, or it may look simplistic or complex. When you need to change to regain simplicity, you must find a way from where you are to where you want to be.
And a related one from Antoine de Saint-Exupery:
Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away
The question of simplicity and good code is not a problem to solve but, rather, something that compels us to reach for more beautiful solutions to different problems — more beautiful than those we already know.
Suppose that Leonardo, after having painted "Mona Lisa", had concluded: "This is the most beautiful painting I can draw, therefore I'll just stick to the style and draw slight variations of her from now on because this is the most beautiful painting ever." He might have sought for something more, too.
Maybe even comment (or color code) the big method to show which objects it was ripped from?
Then maybe something to try and compress this big method and remove lines of code that are only picking out impls at runtime or doing reflection or something?