My first ever lines of Clojure and while the script seems to work as expected, I'm sure it's far from idiomatic Clojure. Thank you for any suggestions.

#!/usr/bin/env bb

; =============================================================================
; USE: ./photos.clj --src [src-dir] --dest [dest-dir]
; * Will copy photos from src-dir to dest-dir, grouped by desired photo format.
; * Indicate desired format by prefixing photo filenames with:
; * - m for small,
; * - v for large.
; * Will notify and fail if you accidentally have multiple photos with the same
; * filename in src-dir.
; =============================================================================

(require '[clojure.tools.cli :refer [parse-opts]])
(use '[clojure.java.io :only (file, copy, make-parents)])

(defn parse-args [args]
  (let [options
          [["-s" "--src SOURCE-FOLDER" "Source folder"]
          ["-d" "--dest DESTINATION-FILDER" "Destination folder"]]
        args (:options (parse-opts args options))
      (assert (:src args) "--src must be specified")
      (assert (:dest args) "--dest must be specified")

(def args (parse-args *command-line-args*))
(defn cathegory [name]
  (let [ch (first name)]  
      (#{\m \M} ch) :small
      (#{\v \V} ch) :large
      :else (assert false (str "Unsupported cathegory: " ch)))))

(defn mark-seen [seen f]
    (assert (not (seen f)) (str "Duplicate file: " f))
    (conj seen f)))

(defn copy-file [seen f]
  (let [fname (.getName f)
        cat (cathegory fname)
        destf (file (str (:dest args) "/" (name cat) "/" fname))]
      (println cat (.getPath f))
      (make-parents destf)
      (copy f destf)
      (mark-seen seen fname))))

(def src-dir (file (:src args)))
(def src-files (filter #(.isFile %) (file-seq src-dir)))
(reduce copy-file #{} src-files)

1 Answer 1

  • fn, defn and let have implicit do, so you can remove it from parse-args, mark-seen and copy-file

  • function parse-args: typo "--dest DESTINATION-FILDER" -> "--dest DESTINATION-FOLDER"

  • function cathegory (typo in the name): you can use destructuring here, case will be better than cond and if you know that your assert has to fail, you can directly throw some error:

(defn category [[ch & _]]
  (case ch
    (\m \M) :small
    (\v \V) :large
    (throw (Exception. (str "Unsupported category: " ch)))))
  • src-dir and src-files don't have to be global variables- I think you can just wrap your reduce in let and put these variables here:
(let [src-dir (file (:src args))
      src-files (filter #(.isFile %) (file-seq src-dir))]
  (reduce copy-file #{} src-files))

You can also use threading macro ->> and write something like this:

(->> (file (:src args))
     (filter #(.isFile %))
     (reduce copy-file #{}))
  • function copy-file: function file has a version with arguments [parent child & more], so if every part of the path is a string, you don't have to join them with "/" manually:
(file (:dest args) (name category) fname)

By the way, Clojure has function cat and you're shadowing it with your variable cat.


Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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