2
\$\begingroup\$

I have made a small script which grabs data into a map from 80 CSV files and calculates some statistics like average, standard deviation etc. It's also adding some additional data to map from filename.

Can you please check if it is a correct way to do so? The idea is to get a list of files in the folder using file-seq, map this to function which reads file lazy, while skipping the first line, convert all text to decimals using read-string, multiply all the data by 1e9 and then calculates all the necessary statistics.

The script works perfectly. I am just concerned about the right style to write programs in Clojure since I am pretty new to it.

Data in CSV files is just numbers in scientific format like "1.721e-9" written in one column.

(def data
  (map (fn [fsc]
     (->> (io/reader fsc)
          (line-seq)
          (rest)
          (map read-string)
          (map #(* % 1e9))
          ((fn [se]
             (let [x se
                   x2 (map #(* % %) se)

                   n (count se)
                   sum-x (reduce + x)
                   sum-x2 (reduce + x2)
                   average (/ sum-x n)
                   variance (-
                              (/ sum-x2 n)
                              (math/expt average 2))]
               (merge {:n n
                       :average average
                       :variance variance
                       :st-dev (math/sqrt variance)
                       :st-dev-sample (math/sqrt
                                        (/ (* n variance)
                                           (- n 1)))}
                      (-> (.getName fsc)
                          (clojure.string/split #"\s")
                          ((partial zipmap [:type :color :voltage :temperature]))
                          (#(assoc % :voltage (read-string (re-find #"[+-]?\d+" (% :voltage)))))
                          (#(assoc % :temperature (read-string (% :temperature))))
                          )))))))

 (->> (clojure.java.io/file "data")
      (file-seq)
      (rest))))
\$\endgroup\$
3
\$\begingroup\$

Don't forget about for

Consider using for instead of map in places where you use an anonymous function with map. This particularly applies to the outer-most map, since with map what you are maping over is sort of hanging by itself at the end of the code.

(def data
  (for [fsc (->> (clojure.java.io/file "data")
                 (file-seq)
                 (rest))))]
    (->> ...)))

In this case in particular, what you are processing is made more clear with for.

Break the code up into (named) functions

In a similar vein to using for, consider breaking the code up into named functions. While anonymous functions are obviously sufficient, a good name can bring a lot of clarity to your code.

At minimum, I would put functions like #(* % %) inside a let or letfn:

(let [square (fn [x]
               (* x x)]
  ...)

But if you think you have a use for the same function in other places, make it a top-level definition:

(defn square [x]
  (* x x))

Consider using fn in favor of #(...)

Personally, I eschew the use of #(...) reader syntax to define functions and just use fn instead. I think the #() syntax was a well-intentioned solution to a largely non-existant problem (verbose anonymous functions), and to the extent anonymous functions are verbose in Lisps, the #() syntax is a poor replacement. It's barely shorter than an equivalent fn form, arguably more difficult to grok (based on the questions I've seen surrounding it on stackoverflow), and has some fundamental limitations (e.g., can't be nested).

Obviously, this is a very subjective view, and certainly not everyone agrees with it.

Use comments to add clarity

Be sure to use comments to provide high-level descriptions of lower-level operations. For example, there are a couple places where you use rest to skip the first element in a sequence. A comment in these places indicating why this element needs to be skipped would go along way.

Likewise, consider adding comments for the major blocks of the code -- i.e., high-level descriptions of each of the loops. Simple one-liners should be sufficient for most of these. You might want a little more description for the mergeing part, though, since that seems to be the core of the logic.

\$\endgroup\$
  • \$\begingroup\$ is the way I used to treat data proper? \$\endgroup\$ – Artem Nov 16 '16 at 9:04

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.