Idiomatic program for someone new to Clojure
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 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/ccb868e0-7e0c-46df-80fc-712f718314e3n%40googlegroups.com.
Re: Idiomatic program for someone new to Clojure
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 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 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/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 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/CAGokn9L65oxePmfJqEDNvyhS9XL-JFjDbQAfk5zdiRctXS_-bQ%40mail.gmail.com.
Re: Idiomatic program for someone new to Clojure
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 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
> 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 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/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 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/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
Re: Idiomatic program for someone new to Clojure
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 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
>> 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
>> .
>
Re: Idiomatic program for someone new to Clojure
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 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
