Comment by barrell
2 days ago
Personally, I would normally reach for loop to check neighboring elements very ergonomically.
(loop [[a b c & more] coll] (recur (apply list b c more)))
There’s also partition if you're working with transducers/threads/list comprehension
(partition 3 1 coll)
Or if you need to apply more complicated transformations to the neighbors/cycle the neighbors
(->> coll cycle rest (map xform) (map f coll))
Using map-indexed to look up related indices is something I don’t think I do anywhere in my codebase. Agreed that it’s not ergonomic
EDIT: those Fibonacci functions are insane, even I don’t understand most of them. They’re far from the Clojure I would advocate for, most likely written for funsies with a very specific technical constraint in mind
Yeah, I guess partition to me always looks like a dangerous tool - for instance I have a sequence of numbers and I want to do a 5 point rolling average
You could do `(partition 5 1 coll)` and then average each element in the resulting seq.. It's very easy to reason about. But I'm guessing the performance will be abysmal? You're getting a lazy list and each time you access a 5 neighbor set.. you're rerunning down you coll building the 5 unit subsets? Maybe if you start with an Array type it'll be okay, but you're always coercing to seq and to me it's hard
Taking the first 5 elements, recurring on a list with the top element dropped is probably better, but I find the code hard to read. Maybe it's a familiarity issue..
Yeah like I said I reach for loop first and foremost. This is what it would look like with comments if it were actually something complicated (although the comments are quite trivial here):
Realistically if performance was a consideration I would probably do:
Should be ~15 times faster to avoid the nested loop. If you want to change the min size it's still pretty clean:
Thank you for the detailed response! Really thought provoking. It wouldn't occur to me to write code like this. It seems like it'd be harder to parse than an imperative index-based solution, but I'm not sure. Do you find it easy to immediately grok? I'm figuring it's just familiarity
- What's the nested loop in the first solution that you've avoided? The `reduce`? the `count`?
- `conj` feels very Lispy (I mean in contrast to Clojure, not C++) .. Isn't it going to have to run down the list every time to add an item?
My outstanding concerns are what I think are the constant coercion to lists/vectors. You also in effect know the result's size, but your runtime/compiler doesn't know that. So you aren't preallocating `memo` and it feels .. wrong haha
Just curious to hear your thoughts :)
Its probably impossible to keep everything so nicely abstract and composable, but I wish it was smoother to just work with arrays, with random access. The current way of dealing with array is always a bit unwieldy in Clojure. And everything coerced to lists. Working with vectors, with mapv filterv etc is helpful, but they don't have random access so it's not always the solution you want.
4 replies →
I thought another option would be to use a `partition-all` transducer (and conj'ing a result list that way), but it seems those don't support step sizes. Maybe that internally could be as efficient as running along iterators in an imperative style
https://ask.clojure.org/index.php/2422/transducer-for-partit...
https://ask.clojure.org/index.php/13187/there-currently-opti...
https://clojure.atlassian.net/browse/CLJ-2797
Hopefully gets added to the language some day..
Exactly. I would use loop or partition+map/reduce for that case. I almost never use map-indexed. In fact I almost never use indexing at all. Mostly, when I have a sequential collection (vector, list, or generic seq), I need to iterate over all the elements, and I’m doing that with map or reduce. IMO, map-indexed has a code-smell that indicates that you’re reaching for an imperative algorithm when perhaps a functional algorithm would be better. Surely, there are times when map-indexed is just what you need, which is why it’s there, but typically not in my experience.
I would agree that map-indexed + *nth* is a code smell. I use map-indexed all the time though, just when I need the index, not other elements in the list