Control structures have been around nearly as long as programming but it's hard for me to see them as more than an annoyance. Over and over again, I find that better code has fewer if-statements, fewer switches, and fewer loops. Often this happens because developers are using languages with better abstractions. They aren't consciously trying to avoid control structures but they do.
If we are working in an object-oriented language, we can replace switch-statements with polymorphism. The same trick works well for if-statements too, but it can be overkill in simple cases. When we use languages with functional features, we can do most of the work that we do in loops using maps, filters, and folds. Control structures end up disappearing, and that can be a good thing.
The problem with control structures is that they often make it easy to modify code in bad ways. Let's take a simple if-statement:
if ...
...
else
...
end
Every place that we have ellipses in that code is a place where we can put more code. Those places can access variables outside of the if. It's very easy to introduce coupling. Moreover, people do routinely nest conditionals inside of conditionals. Some of the worst code I've ever seen is a cavernous nightmare of nested conditions with odd bits of work interspersed within them. I suppose that the real problem with control structures is that they are often mixed with the work. I'm sure there's some way that we can see this as a form of single responsibility violation.
What can we do? Do we have to live with control structures? In general, I think that we do, but it's an interesting exercise to see what we can do to reduce our use of them. Often we can learn new tricks and make our code clearer in the process.
A while ago, I was working on some Ruby code and I needed to write a 'take' function to take elements from the beginning of an array. Ruby already has a take function on Enumerable, but I needed to special behavior. If the number of elements I needed was larger than the number of elements in the array, I needed to pad the remaining space in the resulting array with zeros.
This seems like a job for a simple if-statement:
def padded_take ary, n
if n <= ary.length
ary.take(n)
else
ary + [0] * (n - ary.length)
end
end
Let's look at this code carefully. There's nothing in it which tells us anything about what padding is, and how the pad relates to the array we're making. We can see what is happening if we look close, but we don't see the concepts in the code.
We could introduce some functions to make it clearer, and simplify the conditional by using a guard clause:
def padded_take ary, n
return ary.take(n) unless needs_padding?(ary, n)
ary + pad(ary, n)
end
That's short and sweet but it doesn't take advantage of a simple fact - we can use a null object to get rid of a conditional. An empty array is a wonderful null object. Let's start over.
We don't need a conditional to compute the length of the pad. The length is number of elements we want to take minus the length of the array, if the number we want to take is greater than the array length - it's the maximum of the difference and zero:
pad_length = [0, n - ary.length].max
If we have that, we can pad the array first and then take the number of elements we need from it:
def pad ary, n
pad_length = [0, n - ary.length].max
ary + [0] * pad_length
end
At this point, we can write our padded take:
def padded_take ary, n
pad(ary, n).take(n)
end
What we've done is eliminate an if-statement by forming a computation that always appends a pad. However, sometimes that pad is just an empty array.
I'm not going to argue that this code is simpler than the if-then-else code we started with but it is more declarative and, in general, I don't think that code using this strategy is as prone to abuse.
There's a clear advantage to being able to think at this level of abstraction. It pays dividends when we confront larger problems.
Does this have performance implications? Did you benchmark it at all? How often is this operation being done?
Posted by: Colindean | November 05, 2013 at 07:07 AM
It seems that you recommend using language features, or different languages, to encode control structures in a different way. Logically, the control structures don't seem to have gone. But your way of presenting the logic of program execution is easier to understand and extend. Is that what you're saying?
Posted by: Pattern-chaser | November 05, 2013 at 07:13 AM
I like the idea. I would say that switch statements aren't worse than polymorphism though - they're just different. Switch statements are perhaps better when the number of cases isn't likely to change but the number of functions (which switch on the same type) is likely to change. Polymorphism is better in the opposite situation - the number of cases (or subtypes) is likely to change, but the number of occurrences of the same switch (or number of member functions) is likely to remain constant. Perhaps the same could be said for a simple if statement.
Posted by: Mike | November 05, 2013 at 07:55 AM
Unfortunately, in this example, the code is quite inefficient. Each call to pad makes a copy of the original array. Even in the case where n is ary.length.
I'll write up more about what one could do instead with a language with lazy data structures and lazy evaluation, and whether that is even a good solution anyway.
Posted by: Franklin Chen | November 05, 2013 at 08:10 AM
I like it. The code is expressing the intent in a clear manner. Deriving a proper abstraction, like the `pad` function, requires more thought and time.
In my opinion, the if-statement could be replaced with
pad(ary, n).take(n), and we're done.
We can even rename `pad` to `zeropad`, or something similar.
Posted by: Nick Dandoulakis | November 05, 2013 at 08:21 AM
I'll throw my golf swing into the mix for fun....
def padded_take ary, n
ary[0..n] + [0] * [n - ary.length, 0].max
end
..but that shouldn't take away from your point. Any time you can remove conditions from a function it is a good thing. A mentor once told me that a method should know what it is supposed to do, and should not have to guess at the output based on the inputs. I think this is along the same vein.
Posted by: Nelson | November 05, 2013 at 06:41 PM
RE: "I'm sure there's some way that we can see this as a form of single responsibility violation."
Agreed. More precisely, there are two violations, one of the responsibility rule, one of the open/closed principle. (As you suggest in a recent article, these violations tend to be coupled.)
First, the traditional if/then/else control structure tightly (via syntax) couples the responsibilities of "observing a condition" with "action based on that condition".
Second, we're forced to squeeze all the conditional actions into one block, i.e. the if/then/else actions are open to modification but closed to extension. It isn't a surprise that we end up invasively modifying code to add more actions, rather than extending a condition with more actions.
RE: "What can we do?"
We solve those problems! It isn't difficulty, and answers already exist. My own favorite is based on ArrowChoice in Haskell: a condition is expressed as a sum-type value (a + b), and conditional action is expressed by `left :: (a ~~> a') -> ((a + b) ~~> (a' + b))`.
I favor a more monadic variation that also separates the conditional action from the point of application: `leftM :: ((a ~~> a') * (a + b)) ~~> (a' + b)`.
The technique you're describing is closer to using numbers as conditionals based on `f^4 = f . f . f . f` vs. `f^0 = id`. This isn't open to extension, though, since the number is 'consumed' by this expression form. So I wouldn't recommend it as a general technique.
Posted by: Awelonblue.wordpress.com | November 06, 2013 at 05:31 AM
I think in some cases this is true, but I'm not convinced this is one of them. I think this is what I'd like best:
def padded_take ary, n
ary.take n if n <= ary.length
ary + [0] * (n-ary.length)
end
The example in the article would be better if it hadn't pulled the padding out into its own method, because it hides all the relevant details. With `pad(ary, n).take(n)`, we can't see what it means to pad the array. So we must go down into the next method anyway to answer questions like "does it get padded with zeros or nil?" and "does it pad on the left side or the right side?" and "what if n is negative?"
I think that distracts from the actual point of the article, which is about conditionals, not extracting methods, so the final version should probably be:
def padded_take ary, n
pad_length = [0, n - ary.length].max
(ary + [0] * pad_length).take(n)
end
And for me, at least, it's easier to understand the guard clause than try to think through the normalization that allows the second one to work in both cases.
Posted by: Josh Cheek | November 06, 2013 at 07:43 AM
Consider that in a language with more lazy value support (such as Haskell or Clojure), the solution becomes even more elegant, the equivalent of:
def padded_take ary, n
ary.then(Repeat.new(0)).take(n)
end
where Repeat is an Enumerable which lazily produces an infinite list of some value (in this case, 0), and where #then returns not a lazy Enumerable which will first enumerate the receiver and then, if it runs out of values, enumerate the argument.
You can implement this in Ruby without too much trouble, but in another language these would be part of the standard library, and this idiom would be simple and recognizable enough that defining #padded_take would be silly.
Posted by: Peeja | November 07, 2013 at 05:17 AM
You have not removed conditions. You just pushed them on the side. How do you think max is implemented?
Posted by: Bernard Gallet | November 07, 2013 at 09:13 AM
Bernard: I don't think we can write any software that does not depend upon conditionals at some layer of the stack. But not having them part of the code we directly maintain can give us some advantages.
Posted by: Michael Feathers | November 07, 2013 at 10:01 AM
I like it.
To those saying that calling pad every time is inefficient because it creates a copy when padding is empty: Doesn't this optimization belong in the + operator?
Posted by: Johannes Brodwall | November 08, 2013 at 03:35 AM
Try the APL pogramming language, rarely use loops. Very optimized primitives for processing without loops.
Posted by: Jim | November 08, 2013 at 01:13 PM
Jim: Oddly enough, I'm currently working on a J (APL derivative) interpreter that can be embedded in Ruby: https://github.com/michaelfeathers/jop
Posted by: Michael Feathers | November 08, 2013 at 02:43 PM
"Some of the worst code I've ever seen is a cavernous nightmare of nested conditions with odd bits of work interspersed within them."
Ayende's story is a must here, IMO - http://ayende.com/blog/4612/it-really-happened-legacy-programmers-tales
On-topic, I agree with the enumerable idea; in C#, that would be
ary.Concat(Enumerable.Repeat(0, n)).Take(n).ToArray();
A bit more verbose than Peeja's example but still in the same ballpark, and definitely superior to the if as it expresses the "what" instead of the "how" (take the array, concatenate it with a sequence of zeroes, only keep the first n elements).
Posted by: Marcel Popescu | November 10, 2013 at 12:16 PM
Mikey...
Great point. Meanwhile, though, I was wondering about "ary". I gotta ask, is there a common standard that includes eliminating any repeated characters?
Or, otherwise, what up with that name?
Love,
Mikey
Posted by: GeePawHill | November 10, 2013 at 02:02 PM
Mike: The word 'array' has always had too many letters. I've tried to alert the OED to this, but they haven't responded so I've decided to take matters into my own hands.
Posted by: Michael Feathers | November 11, 2013 at 05:36 AM
Being a mathematics aficionado, I also like these "elegant", "case-less" solutions.
HOWEVER, I have found out that when thinking about such algorithms, almost always we (all stakeholders in the code: analysts, testers, and also these lowly programmers) argue by cases: "Let's assume the list is longer, then ...; but if it is shorter, then ...".
And all the (unit and other) tests done will also do this.
So my experience is that you have gained nothing - nothing at all; and the risk is HIGHER that maintenance introduces errors because each change "changes all cases together".
Yes, if someone writes code where ONLY the cases that behave VERY SIMILARLY are grouped together with such ideas, THEN the code really becomes much better to maintain (I hesitate to write "easier to maintain" ...).
But if the grouping becomes like tricky like in the original example (where the max is just another "case-distinguishing operator", i.e., something, which exactly the same amount of "think time" as an if), then the maintainers will curse you more ....
----------
A special case: In PASCAL (my first language) and all later languages, you can write
boolVar := a > b;
For a long time I and many other "advanced" guys laughed at the people who wrote
IF a > b
THEN boolVar := TRUE
ELSE boolVar := FALSE;
However, after having worked (and instructed) larger teams, I found that the latter allows you do add comments "at the right place":
IF a > b
(* This is the usual case when the user does this and that ... *)
THEN boolVar := TRUE
(* Only when that funny things happens, we arrive at this place; this has a few consequences downstream, namely ... *)
ELSE boolVar := FALSE;
This important "real-world" structure is completely lost in the "elegant", IF-less formulation ...
=======
Nevertheless, refactorings like "replace cases with polymorphism" and also "replace cases with encompassing algorithm" (as in the article) are valuable tools of the trade ...
H.M.M.
Posted by: H.M.Müller | November 11, 2013 at 07:00 AM
(Ahem ... sorry for the 3...5 grammar and spelling errors in my posting).
Posted by: H.M.Müller | November 11, 2013 at 07:05 AM
... after some reflection, I suspect I know why the idea of "unconditional programming" seems so attractive for Michael's padded_take function: In that case, the array in question was used (always or at some places) as a SUBSTITUTE for an infinite list. Thus, the actual job was:
Get me the first n objects of an infinite list defined like that...
There is no case distinction in this specification, and therefore Michael (and we all) expect that none should be in the code. The problem is therefore not with the algorithm, but with the wrong data structure.
Now, the selection of a finite array to represent an infinite list may well have good reasons (among them performance-related onces). And in that case, the "re-introduction" of the case-less abstraction is a good thing.
However, it might also be worthwhile to change the used collection implementation so that one would not get need any case distinction in the first place.
And more important, as I said in my previous comment, there are cases where "the world" has some distinction, and hence that distinction should remain in the code (although much could be said about that "the world" ...).
But Michael was careful enough not to claim that this abstraction is "obviously better" or something like that.
And on the whole, I agree with the statement at the beginning: "Over and over again, I find that better code has fewer if-statements ....".
Posted by: H.M.Müller | November 11, 2013 at 07:46 AM
Actually one can write much simpler version of the function:
def padded_take ary, n
Array.[](*ary, 0 * n).take(n)
end
Posted by: Gregory Shilin | November 13, 2013 at 02:58 AM
Sorry, that's the correct one:
def padded_take ary, n
Array.[](ary, [0] * n).flatten.take(n)
end
Posted by: Gregory Shilin | November 13, 2013 at 03:02 AM
Nice example.
It occurs to me that every time we do this (remove conditionals) is an act of generalisation of special cases: null object, strategy/command, using a map instead of a switch etc.
It gives me the idea to ask myself the explicit question of "how can I change the code so that one case is just a degenerate case of a more general one?"
Reminds me of Kevin Rutherfords ramblings : http://silkandspinach.net/2004/07/16/if/ http://silkandspinach.net/2004/07/16/hexagonal-soup/
Posted by: Johan_alps | January 24, 2014 at 06:48 AM