I’m a beginner and I’m wondering how to write a function in the proper Clojure style. Somehow I ended up writing a function that is basically one let statement with a long chain of bindings and then a single expression. From a coding style standpoint, is there a better way to structure such a function?
I don’t see anything particularly egregious about it. It is verbose, but that does happen.
I myself my trade some vertical space for a less horizontal sprawl (e.g, where you have 3 nested maps. You could also think about using threading macros to remove some of the nesting), but other than that, it does look pretty good.
I agree. Since some of the values calculated early go into the result, this way of stepwise building the result is perfectly fine.
That said, I would probably try to break it apart into some separate functions that each get the ”same” map to work on as it accumulates the needed data. It’s easier to test small nicely named functions in the REPL than a long chain of let bindings.
A pattern I find myself often return to (no idea how idiomatic Clojure it is) is to start with a map and thread it through a pipeline that in turn sometimes consists of pipelines. Something. like so:
The goal I typically aim for is about 20 LOC per function. That seems to be enough pressure to golf the code down and keep the local mental context of a function simple enough to reason about. There are of course exceptions, but it’s been a pretty useful practice. It leads to refactoring “big” stuff into smaller composable functions. It looks like you already did that in this case and you follow the functional style pretty well.
If I had to golf this down a bit, I would look to extract any repeated noise or verbosity into functions or macros where appropriate. Typically functions. I would also reduce the verbosity of the descriptions from the make-this-into-that into this->that, and similarly for constructors, instead of make-a-new-thing into ->a-new-thing. Destructuring can help golf things down and may make it clearing what you’re doing. Also, there may be library functions like map-indexed that do what you want and let you refactor without having to write anything new.
There’s nothing generally wrong with sequential let bindings, although you can try to aim for a more “points free” style where you’re just composing functions (Haskellers do that a bit), but if the intermediate bindings are useful for readability then I’d keep them in. There’s also a style of using anonymous let bindings for imperative stuff, like (let [x 2 _ (println x)] x) which is sometimes more convenient than peppering do while achieving the same result.
On second reading, it seems like you are using records (and apparently typing via the defn-spec stuff). I find that people coming from statically typed languages immediately reach for records and try to use types asap. I would question “why the need for records instead of plain maps at this juncture?” (they certainly have uses, but are less common than generic persistent hashmaps).
One thing I like about the intermediate let bindings is that they come quite naturally when you’re building the function in the REPL. Once you’re done, you can of course refactor them into something more compact if it makes sense, but it does give you nice intermediate steps for debugging, etc.
We tend to lift a lot into names, since Clojure (and lisps in general) is pretty permissive with symbol names given the lack of operators or reserved words. So it’s common to use question marks to indicated predicates, and the convention this->that to indicate transforms or ->thing for a constructor. They’re just names. Similarly, the macros ->, ->>, some->, some->>, as-> etc. are all just naming conventions designed to communicate what’s happening. They aren’t syntax per se (except in the sense that macros extend the language), so there’s no commonality between the -> used there and in the other symbols.
