4
\$\begingroup\$

I'm fairly new to Clojure and looking to improve my use of the proper idioms, and make my code more readable. Here's the problem:

Read from standard input and produce the count of each word to standard output, sorted by word, with a single space between the word and the count. Sort the output by word. Words are A-Z only, for simplicity, and case does not matter (lowercase everything).

My first pass is:

(let [counts (apply merge-with +
  (map #(frequencies (re-seq #"[a-z']+" (.toLowerCase %)))
    (line-seq (java.io.BufferedReader. *in*))))]
  (doseq [[word count] (into (sorted-map) counts)] 
    (println word count)))

It gives the correct output:

$ echo 'One one    two five
>   eight ONE FIVE 
> 
> four OnE two   ' > test
$ java -cp ~/apps/clojure/clojure-1.7.0.jar clojure.main word_count.clj < test
eight 1
five 2
four 1
one 4
two 2

but I have concerns about the code.

  1. Is it too dense? Or is it idiomatic enough? I'm not trying to code golf; this is just how I thought of the problem (I have experience in ML and Lisp.)
  2. I believe the code stays properly within the realm of lazy sequences as much as possible, but would appreciate a Clojure expert to verify or point out where I might be prematurely realizing a lazy sequence, which would make the script unsuitable for large files.
  3. Is (into (sorted-map) counts) the proper idiom for iterating through a map sorted by key?
  4. I find (.toLowerCase s) easier than Clojure's native lower-case because the latter is from a different namespace? Am I anti-Clojure to think that way? Is it part of Clojure culture to prefer the Clojure function to the Java method or does it not matter?
  5. Would it be preferable to rewrite the mapped function as #(->> % .toLowerCase (re-seq #"[a-z']+") frequencies) instead of nesting the calls as I did so? On the one hand, ->> results in fewer parens, but on the other hand might that be a little harder to read? Or is the difference between -> and ->> common knowledge?
\$\endgroup\$
3
\$\begingroup\$
  1. Your solution is great, I found it very readable. You can get a simpler version by treating all the words as a single sequence.

Spoiler alert example:

(for [line (line-seq ...)
      word (re-seq ...)]
  word)

or

(mapcat split-words lines)

then

(sort (frequencies words))

It is nice to keep sequence operations separate from processing.

  1. Yup, good job!

  2. No, just sort the map. (sort counts)

  3. Yes, prefer real functions over interop.

    • Un-hinted interop is a common performance problem
    • interop is less composable
    • built-ins are more portable (to ClojureScript)
  4. Unwrapping deeply nested function calls is good where they are well named, small functions. When they are compound statements it becomes hard to keep mental track of what is flowing through the pipe vs what is flowing through the step. Creating a function spit-words for the (re-seq ...) would make the flow very readable as a thread. Named steps are good too, so it just comes down to whether the names are adding anything, if the threading is too complex, or the steps should be factored out.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.