It would be awesome to receive code review for this Luhn implementation https://github.com/leandrotk/luhn/pull/1/files

@leandrotk100 - some superficial notes: foo.core is only a default because single segment namespaces are problematic, consider using a two-segment ns without the "core" (:gen-class) is a default when making an app, but your code doesn't really implement an app, it's more like a library - it might make things clearer to eliminate the gen-class call and the -main definitions putting the arg list on the same line as the function name doesn't work if you want a doc string or multiple arities, it can't really be used as a consistent style in clojure, so I'd avoid it entirely your loop in double-by-two would be clearer as a reduce, since you are consuming a collection in order clojure.string is not guaranteed to be loaded, if you use it, require it and use :as like you would with any other ns pretty much always, use [foo] instead of (list foo) - there are very few cases where list actually makes sense in clojure code flatten is an antipattern, you know enough about the incoming structure (it's not coming from an external source), so you can use eg. (apply concat ...) which is more specific about the shape of the input (= x 0) should always be (zero? x)

regarding the apply concat instead of flatten, an even more elegant solution there is for the providing function that generates the data to use mapcat instead of map

hth

Feel free to add comments in the PR if you want it

I will take a look at your comments!

Thanks @noisesmith! 🙂

oh - and I missed this on my first shallow read: you repeat the ns form twice, that doesn't hurt anything but it's definitely weird

> putting the arg list on the same line as the function name doesn't work if you want a doc string or multiple arities, it can't really be used as a consistent style in clojure, so I'd avoid it entirely Hey @noisesmith, do you mean that you prefer:

clojure
(defn foo
 []
 :foo)

;; instead of
(defn foo []
  :foo)

right - because as soon as you have multiple arities or a doc string, putting the arg list on the same line is awkward

and it's better to have something consistent

Yeah good point. Also useful when diffing the commits. Thanks

I will accept a special case for (defn foo [] :foo) which I would put the entire thing on a single line

I think it's common to put them in the same line until you have a docstring or multiple arities, at least that's what I've seen so far

just to name an example https://github.com/mmcgrana/ring/blob/master/ring-devel/src/ring/middleware/stacktrace.clj