Fork me on GitHub
#code-reviews
<
2018-02-25
>
Charles Fourdrignier07:02:48

Hello everybody! After writing some advent of code solutions and playing with some other Clojure code. I think it's time for me to learn by doing something "real". I write a small scrapper to check my "library borrowings". https://github.com/Charlynux/library-monkey At the moment, the code is pretty small (100 lines) and straightforward (no error-handling, no tests,...). It works, but I would appreciate some review of my code. All advices/critics are welcome. (A more experiment clojurian can probably point some "not the Clojure way here" or "too much code there".)

sveri16:02:05

@charles.fourdrignier Hi, a few notes from a first look:

sveri16:02:28

You should use a formatter, the lines in your imports are not aligned. If you use the ->> or -> macro and call a function with only one argument you can omit the parantheses like so: (-> (foo bar baz) keyword)

sveri16:02:25

Dont use use for imports, instead prefer require :as In html_parse.clj on l.21 you start with the data as a first argument to the threading macro. In l.35 you dont do that but instead start with the function and data as argument. I think it doesnt matter which style you choose, but I think its important to be conistent at least

sveri16:02:21

I dont like the formatting of network l.14-20, this could be more compact and I would make the URI a var. The same in get-borrowings

sveri16:02:48

As I see this often, coming from a java world I find it weird that namespaces are used so seldom. Please see here for the convention for java packages: https://docs.oracle.com/javase/tutorial/java/package/namingpkgs.html

sveri16:02:09

And last but not least, no tests 😉

Charles Fourdrignier18:02:38

Thanks a lot ! I will update my code with that.

sveri19:02:37

@charles.fourdrignier no problem, I am also open to discussion, of course