The Iterate-and-Mutate Programming Anti-Pattern
dogsnog.blog
dogsnog.blog
There have been a number of times I've seen junior developers who are high on functional/immutable paradigms commit mazes of mapping, filtering, rolling and unrolling, resulting in something that's hard to understand and hard to debug.
Even in the example here, while it's obvious what the replacement code _does_, it's less clear what it's _intending_. I actually had to refer back to the original code to understand that the thing we were trying to generate were IDs of some sort.
I would need at least 3 more pieces of data -- the other (experience_level, programming_style, bug_rate) tuples -- in order to judge whether this constitutes a pattern which is best avoided.
Everything can be abused. I see people doing side effects in their mapper callbacks, deeply nesting ternaries and switch and if/else statements, mutating far off objects, and just generally making code way more complex than it needs to be. Usually, this is from junior devs, but not always.
In the right hands, functional programming in JS can be very powerful. It can be easier to reason about (linear data flow, declarative code), and more performant (immutable data structures, transducers). In the wrong hands, it can be even more opaque than imperative, procedural code.
It's also a little bit of Paul Graham's fault with his silly accumulator challenge. Not every language needs closures, and because closed over variables act as global variables, they might even undo the progress we've made in adopting structured programming. But by framing accumulator-capable languages as being more "powerful" now nobody wants to forgo them.
This blogpost does not even attempt such an enumeration, but instead appeals to aesthetics.
FWIW I went through a period of using map and filter a little bit, went, "that's nice", and then resumed using for loops, sometimes with comprehension syntax to sugar it up. Both methods basically cover ways to specify simple iteration and selection. But I know which one is going to port better across the majority of environents: the plain for-next, and if a complex iteration is called for, the while loop. It puts the data allocation where I can see it, and it maps to the single-threaded computing model that is still the default today.
If I want a more complex query, I am going to start wanting a more expressive query language than either imperative or functional selection and iteration can provide by themselves. Left outer joins do not come easily to either method. Neither does constraint logic programming. You obviously can implement those things, but it isn't blindingly obvious, and when your problem grows to need a very broad expression of selection and iteration, it is those kinds of tools that you really need to aim for.
- For loops are evil
- Lambda overhead doesn't exist
- Nor does the overhead of looping over the same thing N times
Whilst I kind of broadly agree with the article, sometimes you just need a for loop.
Take the example of Java:
Why are loops faster than using streams (in most cases)? Shouldn't the optimizer/jit be able to see that most streams (even chains of map, filter and reduce etc) are equivalent to simple loops and just optimize away the overhead (in the cases where loops are (obviously) faster; non-parallell, serial computing)?
[0] https://www.cs.cmu.edu/Groups/AI/html/cltl/clm/node347.html
And C++ (and rust probably) compilers have no issue doing these optimizations as well.
A significant chunk of programmers decided that for loops are evil, even for loops with a 3 lines body, and that imperative programming languages should in general become functional, because their imperative nature is actually evil.
And I also even broadly agree in theory, but in practice there are consequences, especially in languages that were not designed with a strong focus for that to begin with; or even just with a mismatching culture of most programmers using that language, etc.
So why don't people just use a functional programming language if they think that is a panacea? In most case this will yield a far better functional programming experience...
Also, we are going nowhere if we base all our practice on random soft opinion pieces. We should not blindly believe claims like "An explicit for loop is another common source of bugs" without serious evidences.
but this is about looping and appending to a new (already empty) array, and he calls that an anti-pattern...
im not so sure, seems more like "i like this style, so eveything else is an anti-pattern"
He's just working in wretched language without macros.
TXR Lisp:
(build
(each ((i (range 1 5)))
(let ((id `car-@i`))
(when (car-empty carid)
(add id)))))
The build environment provides deque semantics. Items can be added on both ends, and also deleted. A depth first search can be succinctly expressed using the closely related buildn:
;; breadth-first traversal of nested list;
(defun bf-map (tree visit-fn)
(buildn
(add tree)
(whilet ((item (del)))
(if (atom item)
[visit-fn item]
(each ((el item))
(add el))))))
(loop for i from 1 to 5
for car-id = #?"car-${i}"
when (car-is-empty car-id)
collect car-id)That isn't happening. The loop is over an integral range (1..5 in both examples). That integer is being hashed in the example and the hash is being used for a SIDE EFFECT. Internally, the select() is doing the exact same thing, but there is no label on the value and it's being returned out of the ostensible container function.
This is not a good example of why this pattern is bad nor of why side effects are bad.
I do agree with what you are saying, don't modify the enumerable you loop over. IIRC C# is pretty safe in that it doesn't allow modifications to the enumerable inside of a for loop or foreach loop.
I think it's interesting people say functional constructs like map don't involve looping. They do, it's just hidden from the caller of those functions.
On the other hand, if you do need the detailed control, the terse version doesn't let you have it, so you have to use the verbose version.
One hides the loop (potentially doing more work because of it) and one doesn’t.
The big impact as I see it is that it removes non-important bits and keeps you from messing those up, namely picking the right variable and indexing. Each new lambda in the chain is scoped to it's inputs, not all previous inputs in the iteration.
Which is to say if you get the implementation details wrong you’ve got the implementation wrong and notice very quickly with any halfway reasonable test.
Of course the same is true in the for-loop example. But overall, there are more things to specify in the for-loop:
let car_ids = [];
for (let i = 1; i <= 5; i++) {
const car_id = `car-${i}`;
if (car_is_empty(car_id)) {
car_ids.push(car_id);
}
}
return car_ids;
- storage location for result
- loop variable
- loop variable initial value
- terminating condition (is it < or <=?)
- increment
- temp variable for producing ids
- test condition for filtering
- how to add new item to result
return (1..5)
.map{|n| "car-#{n}"}
.select{|m| car_is_empty(m)}
- min value of range
- max value of range (are ranges inclusive or exclusive?)
- how to generate id
- how to query
Now, this is a trivial example. I wouldn't argue that either way is clearly better than the other in this particular case. But, in my experience and opinion, the latter is much better as things become more complex.
Particularly when on one side of things you put inclusive and exclusive together with the max value of the range and on the other you separate them.
IME this is basically horses for courses, some people will be at home with procedural syntax and other prefer using functional concepts.
Things can get tricky if you start modifying the container you're iterating, especially with deletions. OTOH iterators can make this impossible (e.g. through borrowing in Rust) or at least detect "iterator invalidation" at runtime.
In certain programs it's also easy to modify the container by accident, e.g. if you call a function that triggers an event that indirectly modifies your container.
If you do arithmetic on iteration variable, especially if the loop body has conditionals, then it's harder to follow the code, and easier to make off-by-one errors or infinite loops.
Not every loop is clearer with iterators, but some idioms like `.filter()` or `.any()` are clearer and even shorter to write.
A good argument for the second example is readability. The first one re-invents map & filter which are well known and quickly understood.
> Code that mutates (changes) a variable is harder to reason about, and so easier to get wrong.
This is true when the mutation is "off the page" - in some other scope - but there is no mutation going on in this example that is hard to understand.
> An explicit 'for' loop is another common source of bugs.
The C-style 'for', where one manipulates a control variable which nominally denotes those elements of a set which one is concerned with, is very prone to error because of this indirection, but both versions of this code do this!
> The original snippet is longer because it’s doing boring, low-level work. More, boring, code is another source of bugs.
Maybe, but it is not demonstrated here. 'Boring' is an odd adjective here: how often have you seen the source code for a complete application, let alone anything larger, that you just can't put down? real-world code is about as exciting as a balance sheet, and for good reason.
> Long code with low-level operations like for loops and pushing values onto arrays is not expressive.
Expressiveness might be a feature of a language, but it is not something programs inherit merely by being written in a particular language. Programs are not poetry, and what we want from them is clarity. Expressiveness may help with that, but this example does not demonstrate it (maybe the author should have given us an example in APL?)
> The shorter, functional version has no variables, no mutation, and no explicit looping. It’s much shorter and easy to understand, once you’ve seen this style.
More of the same, with a touch of the Emperor's New Clothes thrown in to suggest that if we do not get it, we are just not sophisticated enough to understand.
Don't get me wrong: mutation is the cause of a lot of problems, and I would prefer for procedural languages to have variables const/immutable by default, but this sort of article just does not make the case. There must be several thousand articles like this written every year, all pretty much the same, with the author never asking whether their example actually makes the case they are claiming (I would guess because they are so convinced that they are right, they don't ask the question, and the existence of so many prior articles of the same sort (which they are effectively plagiarising) makes it seem obvious.)
To be fair, a decade ago I was in complete agreement with the precursors to this article.
One use-case is when you lack proper iterators. In JavaScript, the map()/filter() version can be an order of magnitude slower because it's creating and copying a new array at each step. This can be a worthy trade-off for readability in certain cases, but in hot paths converting to the iterate-and-mutate version is one of the most common optimizations I go in and make later on.
Another use-case is readability (sometimes). reduce(), in particular (going from a collection of values to a single value), is notorious for being hard to follow. Of course mapping and filtering are known for usually being very intuitive. But as a baseline, a step-by-step for loop is always a certain degree of kind of intuitive. YMMV, but there are cases where it's the more readable option.
It's probably still worth identifying this as an anti-pattern, so long as you qualify "anti-pattern" with the fact that tradeoffs always need to be considered in context.
Hot paths aren’t the common case. You should always default to the more readable syntax (map/filter) and then refactor once you’ve determined where your bottlenecks are.
Transducers are also an option to get the map/filter syntax without losing and performance, but I think most will unfortunately shy away from those.
I do agree that reduce is over used. Reducers as a pattern is really powerful (with transducers), but no one uses them right.
That's exactly what I said:
> in hot paths converting to the iterate-and-mutate version is one of the most common optimizations I go in and make later on
> Transducers are also an option to get the map/filter syntax without losing and performance
All you need are iterators. Python, Rust, Haskell, most languages support performing these functions with iterators. There are even iterator libraries for JS, if you don't mind an extra dependency. But vanilla JS does not support this.
Still, being reasonably functional can have surprisingly good effects despite extra allocations / iterations.
Java lambdas do have instantiation cost but I'd wager that for the majority of my code low-cost data parallelism beats that. Also, a lot of the theory here is that the JIT could kill off the instantiation and convert to just calls. Why doesn't it do that?
Rust, perhaps?
1. Long, unreadable, and undebuggable (since you can't step through them)
2. Ones you wish could have just been a Python list comprehension:
return [f"car-#{n}" for n in range (1,6) if car_is_empty(n)]What is this "select" function, why isn't it called filter? I actually don't know how to Google this because the HTML select gets in the way.
I'd really do it in two lines, because I want to get the first half right and then the second, and be able to inspect the first half independently of the second while debugging. The pipelines don't let you do that.
car_ids = [f"car-#{n}" for n in range (1,6)]
empty_car_ids = [id for id in car_ids if car_is_empty(id)] Why write all this async javascript? use PID ! {msg}You could say reduce can work but ultimately reduce needs to be implemented using recursion or a for loop. Reduce is a higher level function and therefore not part of the argument.
That being said I do know about Tail recursive optimization. Most language implementations do not support that though.