Idiomatic program for someone new to Clojure

[James Lorenzen]

Hello all,

This is my first Clojure program and I was hoping to get some advice on it since I don't know any experienced Clojure devs. I'm using it locally to print the latest build numbers for a list of projects.

```

(ns jlorenzen.core

  (:gen-class)

  (:require [clj-http.client :as client])

  (:require [cheshire.core :refer :all]))

(defn fetch-pipeline

[pipeline]

(client/get (str "https://example.com/go/api/pipelines/" pipeline "/history")

{:basic-auth "username:password"}))

(defn get-latest-build

[pipeline]

(let [response (fetch-pipeline pipeline)

json (parse-string (:body response) true)]

(let [[pipeline] (:pipelines json)]

(:counter pipeline))))

(def core-projects #{"projectA"

"projectB"

"projectC"

"projectD"})

(defn print-builds

([]

(print-builds core-projects))

([projects]

(let [builds (pmap #(str % " " (get-latest-build %)) projects)]

(map #(println %) (sort builds)))))

```

This will output the following:

```

projectA 156

projectB 205

projectC 29

projectD 123

(nil nil nil nil)

```

A few questions:

• How can this program be improved?
• How idiomatic is it?
• How can I prevent it from returning the nils at the end? I know this is returning nil for each map'd item; I just don't know the best way to prevent that.

Thanks,

James Lorenzen

[Justin Smith]

a small suggestion: you don't need to nest let inside let, a clause
can use previous clauses:

(defn get-latest-build
[pipeline]
(let [response (fetch-pipeline pipeline)
json (parse-string (:body response) true)

[pipeline] (:pipelines json)]
(:counter pipeline))))

also consider using get-in:

(defn get-latest-build
[pipeline]
(let [response (fetch-pipeline pipeline)
json (parse-string (:body response) true)]

(get-in json [:pipelines 0 :counter])))

finally, this can now be simplified into a single threading macro:

(defn get-latest-build
[pipeline]
(-> (fetch-pipeline pipeline)
(:body)
(parse-string true)
(get-in [:pipelines 0 :counter])))

> --
> You received this message because you are subscribed to the Google
> Groups "Clojure" group.
> To post to this group, send email to clo...@googlegroups.com
> Note that posts from new members are moderated - please be patient with your first post.
> To unsubscribe from this group, send email to
> clojure+u...@googlegroups.com
> For more options, visit this group at
> http://groups.google.com/group/clojure?hl=en
> ---
> You received this message because you are subscribed to the Google Groups "Clojure" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clojure+u...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clojure/ccb868e0-7e0c-46df-80fc-712f718314e3n%40googlegroups.com.

[Brandon R]

Hey James,

Another small suggestion is you can just pass println to map, since it takes 1 argument in your case.

(map println (sort builds))

But here, since you just want to perform side effects, maybe run! would be a better function to use.

(run! println (sort builds))

This would cause it to return just one nil. Clojure is a functional language, and every function returns a value. You'll see the return in your REPL, but if the program is run in another way, such as packaged as a jar, you would just see the print output.

- Brandon

To view this discussion on the web visit https://groups.google.com/d/msgid/clojure/CAGokn9L65oxePmfJqEDNvyhS9XL-JFjDbQAfk5zdiRctXS_-bQ%40mail.gmail.com.

[James Lorenzen]

Very cool everyone. This is exactly the kind of feedback I was hoping for. I'm going through Clojure for the Brave and I hadn't made it to the macros chapter yet. That single threading macro is pretty sweet!

Thanks everyone!

[aditya....@gmail.com]

I'd try to separate the "I/O or side-effecting" parts from the "purely data processing" parts. This makes the program much easier to test --- the "purer" the code, the better it is. This also helps tease apart domain-agnostic parts from domain-specialised parts, which is useful, because domain-agnostic parts tend to be generalisable and thus more reusable.

I'd also avoid mixing lazy functions like `map` with effectful things like `GET` requests, because : https://stuartsierra.com/2015/08/25/clojure-donts-lazy-effects

I've taken the liberty to rearrange the code, and rename functions to illustrate what I mean.

(defn pipeline-api-endpoint ;; lifted out of `fetch-pipeline` 

  [project-name]

  ;; knows how to translate, or perhaps more generally, map, a project name to a project URL format

  (str "https://example.com/go/api/pipelines/" project-name "/history"))

(defn http-get-basic-auth ;; instead of `fetch-pipeline', because now this operation doesn't care about a specific type of URL  

  [well-formed-url] ;; it can simply assume someone gives it a well-formed URL.

  (client/get well-formed-url

              {:basic-auth "username:password"})) ;; we'll see about this hard-coding later

(defn pipeline-build-count ;; now this only cares about looking up build count, so no "GET" semantics

  ;; assumes it gets a well-formed response

  [{:keys [body] :as response}] ;; destructuring for convenience and function API documentation

  (-> body

      (parse-string true)

      :pipelines

      first

      :counter))

(defn fetch-pipeline-counts! ;; ties all the pieces together

  [project-names]

  (reduce (fn [builds project-name] 

   ;; uses reduce, not map, because: https://stuartsierra.com/2015/08/25/clojure-donts-lazy-effects

            (conj builds

                  (-> project-name

                      pipeline-api-endpoint

                      http-get-basic-auth

                      pipeline-build-count)))

          []

          project-names))

Now... It turns out that fetch-pipeline-counts! is a giant effectful process, tied directly to http-get-basic-auth. We could try to lift out the effectful part, and try to make it a pure function.

(defn http-get-basic-auth

  [well-formed-url username password]

  (client/get well-formed-url

              {:basic-auth (str username ":" password)}))

(defn http-basic-auth-getter

  "Given basic auth credentials, return a function that takes an HTTP endpoint, and GETs data from there."

  [username password]

  (fn [well-formed-url]

    (http-get-basic-auth well-formed-url

                         username

                         password)))

(defn fetch-pipeline-counts-alt

  [pipeline-fetcher project-names]

  ;; Easier to unit test. We can pass a mock fetcher that doesn't call over the network.

  ;; In fact, we can now use any kind of "fetcher", even read from a DB or file where we may have dumped raw GET results.

  (reduce (fn [builds project-name]

            (conj builds

                  (-> project-name

                      pipeline-api-endpoint

                      pipeline-fetcher

                      pipeline-build-count)))

          []

          project-names))

(comment

  (fetch-pipeline-counts-alt (http-basic-auth-getter "username" "password")

                             ["projectA"

                              "projectB"

                              "projectC"

                              "projectD"])

 )

A closer look might suggest that we're now describing processes that could be much more general than fetching pipeline counts from an HTTP endpoint...

Enjoy Clojuring! :)

[James Lorenzen]

Thanks for the suggestions aditya. I definitely like where you are headed. I have a few questions. This syntax in `pipeline-build-count` method looks confusing to me:

> [{:keys [body] :as response}] ;; destructuring for convenience and function API documentation

Would you mind breaking that down a little more?

Also, your program works great though it only returns a list of the latest build numbers without the project name. I'm trying to think of how I could include that but I'm not seeing it. The fetch pipeline response does include the project name so I could just return it and the counter; so I could return a tuple? or just two values (I think clojure can support that). I'd rather just thread the project-name through though.

Thanks for all the help! I'm learning a ton.

[James Lorenzen]

So I think I can answer my first question. That particular line in `pipeline-build-count` is doing associative destructing. And you didn't have to do the `as response` but like you said in your comment, you are just doing that to sort of document the method. That sound about right?

[Peter Strömberg]

Seconding aditya's advice here. I can truly recommend watching this talk about solving problems the Clojure way:

https://youtu.be/vK1DazRK_a0

Interestingly, he applies it on a JavaScript code example. Which makes it all that much more powerful. 

To view this discussion on the web visit https://groups.google.com/d/msgid/clojure/33bbba06-2eae-4e2d-8bf2-7322fe7d4db5n%40googlegroups.com.

[aditya....@gmail.com]

On Wednesday, December 16, 2020 at 3:12:22 AM UTC+5:30 jamesl...@gmail.com wrote:

So I think I can answer my first question. That particular line in `pipeline-build-count` is doing associative destructing. And you didn't have to do the `as response` but like you said in your comment, you are just doing that to sort of document the method. That sound about right?

Yes it's "associative destructuring". The main purpose of destructuring is to conveniently bind a name to a value found somewhere inside a data structure. The additional benefit is that it also serves to document the function API, which IDE tooling can surface.

The term "destructuring" sounded heavy when I first tried to grok it. It got easier when I started thinking of it like visually matching shapes (of name bindings) to shapes (of data). Sort of like a fancy mask or a cookie cutter pattern. To illustrate:

;; If I see this:

(let [[a b c & others]
      [1 2 3 4 5 6]]
  ;; do something
  )

;; I visualise the effect of destructuring as:

1  2  3    4 5 6
|  |  |   (     )
v  v  v      v
[a  b  c & others]

 

On Tuesday, December 15, 2020 at 2:33:08 PM UTC-6 James Lorenzen wrote:

Also, your program works great though it only returns a list of the latest build numbers without the project name. I'm trying to think of how I could include that but I'm not seeing it. The fetch pipeline response does include the project name so I could just return it and the counter; so I could return a tuple? or just two values (I think clojure can support that). I'd rather just thread the project-name through though.

There are at least two ways to do this. In both cases, I'd favour returning a hash-map, instead of tuples.

One way is to modify `pipeline-build-count` like this:

(defn pipeline-build-count
  [{:keys [body] :as response}]
  {:project-name (:the-key-for-project-name body)
   :latest-build-count (-> body

                           (parse-string true)
                           :pipelines
                           first
                           :counter)})

The other way is in the fetch-pipeline-counts function. Something like this:

(defn fetch-pipeline-counts-alt
  [pipeline-fetcher project-names]
  (reduce (fn [builds project-name]
            (conj builds
                  {:project-name project-name
                   :latest-build-count (-> project-name
                                           pipeline-api-endpoint
                                           pipeline-fetcher
                                           pipeline-build-count)}))
          []
          project-names))

If the name is the only relevant detail that matters, then either way is fine. However, I'd modifying the pipeline-build-count function, because it has access to all the information about a pipeline, and we may want to process and/or pass through more of it later.

 

Thanks for all the help! I'm learning a ton.

Cheers :)

[Derek Troy-West]

Hello James,

 Aditya links to one of Stuart Sierra's Do's and Don't posts.

That series of posts really influenced my basic Clojure style and I'd suggest reading them all: https://stuartsierra.com/tag/dos-and-donts, they're (mostly!) great and simple advice.

Best,

 Derek

[Jakub Holý]

I am late to the party but you might find https://blog.jakubholy.net/clojure-common-beginner-mistakes/ useful. It also links to a few very valuable resources.