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! :) On Monday, December 14, 2020 at 10:51:52 PM UTC+5:30 jamesl...@gmail.com wrote: > 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! > > On Monday, December 14, 2020 at 11:00:02 AM UTC-6 brando...@gmail.com > wrote: > >> 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 >> >> On Mon, Dec 14, 2020 at 8:42 AM Justin Smith <noise...@gmail.com> wrote: >> >>> 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]))) >>> >>> On Mon, Dec 14, 2020 at 7:18 AM James Lorenzen <jamesl...@gmail.com> >>> wrote: >>> > >>> > 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 >>> > >>> > -- >>> > 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 >>> . >>> >>> -- >>> 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/CAGokn9L65oxePmfJqEDNvyhS9XL-JFjDbQAfk5zdiRctXS_-bQ%40mail.gmail.com >>> . >>> >> -- You received this message because you are subscribed to the Google Groups "Clojure" group. To post to this group, send email to clojure@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+unsubscr...@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+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/clojure/ec3eb75e-4852-4525-9bb6-2a1e6baa4375n%40googlegroups.com.
