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.
On Monday, December 14, 2020 at 11:35:15 PM UTC-6 aditya....@gmail.com 
wrote:

> 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/7e3a78c6-5e8b-47e0-b93b-40faa261f6fen%40googlegroups.com.

Reply via email to