code-reviews

Idan Melamed 2021-01-09T10:07:09.207600Z

https://github.com/idanmel/tic-tac-clojure/ After a couple of years of trying to stick with learning Clojure, I created a tic-tac-toe "game engine". Any feedback would be appreciated. πŸ™‚

jaihindhreddy 2021-01-10T13:17:48.209100Z

Instead of This is the only function that should be used from outside this namespace..., you can use defn- to make fns private to the namespace.

πŸ‘ 1
pavlosmelissinos 2021-01-11T21:26:59.209300Z

Also even though this is a pretty contained project and you don't have a lot to gain, on top of replacing most defns with defn- (or instead of it) you can have a separate api namespace just for symbols that are meant to be exposed to the world

πŸ‘ 1
pavlosmelissinos 2021-01-11T21:35:29.209600Z

I'd also add some specs and try to use them to validate the user's input (e.g. move) to find out if/how I could "reduce" the boilerplate in the turn function

πŸ‘ 1
Idan Melamed 2021-01-12T09:19:54.210400Z

Thanks everyone, I'll make the recommended changes.

Idan Melamed 2021-01-12T09:23:21.210600Z

@jaihindhreddy @pavlos Coming from Python, it will not common practice to use private functions. Is it common practice to use defn- in Clojure?

jaihindhreddy 2021-01-12T09:26:21.210900Z

@idanmel In my experience, what's more common, is for projects to have entire namespaces with impl in their name, and no promises made to the consumer. This way might be a little cleaner. But you do tend to see defn- too. Also, there is no enforcement as such. You can still call private fns from other namespaces (, you'll get warnings of course).

πŸ‘ 1
Idan Melamed 2021-01-12T09:36:39.211100Z

@jaihindhreddy "namespaces withΒ `impl`Β in their name" Not sure what that means, and google comes up short for me. Where can I read about it?

jaihindhreddy 2021-01-12T09:46:14.211400Z

@idanmel For an example of this, https://github.com/tonsky/datascript/tree/master/src/datascript. The impl folder contains namespaces that are to be considered implementation detail by consumers. You can also put :private true https://clojure.org/reference/metadata on namespaces too (like defn- does to functions), but I don't think that does anything.

Idan Melamed 2021-01-12T09:50:00.211600Z

Thanks @jaihindhreddy, I'll look into it!

Idan Melamed 2021-01-12T13:19:56.211800Z

@jaihindhreddy, I implemented the impl. Thanks πŸ™‚

πŸ™Œ 1
enforser 2021-01-09T16:35:54.208300Z

One thing I noticed is that your docstrings should be before the function parameters. Anything after the function parameters is evaluated, so each function is actually initializing the string the immediately forgetting about it

πŸ‘ 1
πŸ˜… 1
1
enforser 2021-01-09T16:37:32.208500Z

in draw-status you can use when instead of if

πŸ‘ 1
enforser 2021-01-09T16:39:16.208700Z

A style I prefer is to destructure right in the parameters.. i.e. instead of

(defn f [foo]
  (let [{:keys [a b]} foo] ...))
you could just do:
(defn f [{:keys [a b]}] ...)

πŸ‘ 1