code-reviews

alice 2019-02-28T00:07:55.001600Z

Hey guys, I just made this thing https://github.com/fp-alice/cljmcs Aside from not having any tests, am I doing anything wrong?

2019-02-28T00:30:42.002500Z

this could be expressed as (set/rename-keys x {:download :file :href :location}) https://github.com/fp-alice/cljmcs/blob/master/src/cljmcs/http/scraper.clj#L11

alice 2019-02-28T00:31:11.002900Z

never seen that function before, good tip

2019-02-28T00:33:38.003900Z

subjectively, sometimes (first (filter f coll)) is better than constructing a predicate that returns its input so you can use some https://github.com/fp-alice/cljmcs/blob/master/src/cljmcs/minecraft/servers.clj#L16

2019-02-28T00:34:47.004900Z

the predicate there could be (comp #{minecraft-version} :version) - returns truthy if the value under the key matches any value in the set

2019-02-28T00:34:57.005100Z

(that's also subjective)

alice 2019-02-28T00:35:42.005300Z

How does that work?

alice 2019-02-28T00:35:50.005700Z

i understand compose

2019-02-28T00:35:54.006100Z

a keys is a function from a map to the value under the key

2019-02-28T00:36:07.006600Z

a set is a function from a potential member to itself only if contained in the set

2019-02-28T00:36:29.007200Z

(#{foo} foo) -> foo

alice 2019-02-28T00:36:49.007900Z

Oh so it's like getting the version and then checking if it's equal to the given one with the set

2019-02-28T00:36:53.008100Z

(#{foo baz} bar) -> nil

2019-02-28T00:36:57.008400Z

right

alice 2019-02-28T00:36:58.008500Z

I always read comp backwards

2019-02-28T14:45:40.000100Z

I had this same problem. But apparently every functional language that has compose does it from right to left.

2019-02-28T00:39:22.009Z

yeah, if the devs in your team (you) don't find that straightforward, it's probably a bad style choice

2019-02-28T00:39:29.009300Z

but it's an idiom I personally like

alice 2019-02-28T00:39:51.009600Z

I love function composition I just always have to read the sexps twice

2019-02-28T00:40:49.010300Z

you use http://clojure.java.io here already, so you could use io/file, but you could also import java.io.File so that you can just use File https://github.com/fp-alice/cljmcs/blob/master/src/cljmcs/os/files.clj#L15

2019-02-28T00:42:13.011100Z

this could be clojure.string/join https://github.com/fp-alice/cljmcs/blob/master/src/cljmcs/os/files.clj#L8

alice 2019-02-28T00:42:13.011200Z

oops, io/file makes sense

alice 2019-02-28T00:42:28.011700Z

that too

2019-02-28T00:44:27.012600Z

the portable version of this is (System/getProperty "user.dir") https://github.com/fp-alice/cljmcs/blob/master/src/cljmcs/os/shell.clj#L8

alice 2019-02-28T00:44:56.013Z

If I want to make it work on windows too I'll keep that in mind thanks for the tip

2019-02-28T00:45:44.013800Z

an extra "print-generic-help" type thing as the odd numbered element at the end of the case will make more sense to the user than the runtime error clojure gives if no case is matched https://github.com/fp-alice/cljmcs/blob/master/src/cljmcs/core.clj#L14

2019-02-28T00:46:04.014400Z

well, the getProperty is also a single method call, instead of a chain of functions as well

alice 2019-02-28T00:46:17.014600Z

good point

alice 2019-02-28T00:46:36.015100Z

thanks for the criticism it's really valuable to me 🙂

2019-02-28T00:46:54.015400Z

it's all my opinions so far, no bugs

2019-02-28T00:48:01.016600Z

in fact this

(if action
      (case action
        "download" (download-command/download! arguments)
        "list"     (list-command/list! (first arguments))
        "help"     (help-command/help!))
      (help-command/help!))))
could be (case action "download" (download-command/download! ...) "list" (list-command/list! ...) "help" (help-command/help!) (help-command/help!)) - both the lack of an action and an unrecognized action often map to the same help output in shell utils

alice 2019-02-28T00:48:35.017100Z

Didn't think of that

2019-02-28T00:49:50.017800Z

or for style you could differentiate the help output from unmatched by adding a prefixing "no matching command"

jaihindhreddy 2019-02-28T02:41:16.018500Z

Whenever I look at (comp f g h), I tend to see #(f (g (h ...))). Of course if you transform it to #(-> ... f g h) it will seem backwards.