7
\$\begingroup\$

I am implementing Unix sort in Clojure. This is my first program, and I would like some feedback regarding any non-idiomatic, harmful code and any other best practices I can include in future versions. I tried my best to follow the coding standards, but went with camelCase instead of hyphen-case for variable and function names.

Main class: sort.core

(ns sort.core
  (:gen-class)
  (:use clojure.contrib.core)
  (:use clojure.tools.cli)
  (:require [sort.lib :as lib]))


(defn -main [& args]
  (let [[options args banner] (cli args    
                                   ["-h" "--help" "Show help" :default false :flag true]
                                   ["-i" "--input" "The input file" :default nil]
                                   ["-d" "--delim"  "The delimitter" :default ","]
                                   ["-f" "--field" "The column to parse(1 indexed)" :parse-fn #(Integer. %)])
        {:keys [delim field input]} options]


    (lib/help options args banner)

    (when (= (lib/fileKind input) "file")
        (lib/printOutVector delim
                            (lib/sortVector field
                                            (lib/splitFile input delim ))))

    (System/exit 0)))

Library: sort.lib

(ns sort.lib
  (:require [clojure.string :as str])
  (:import java.io.File))


(defn splitFile 
  "splits a file into a 2d array" 
  [s delim] 
  (pmap #(str/split % (re-pattern delim)) 
        (str/split(slurp s) #"\n")))


(defn printOutVector
  "prints out the 2dVector formatted by the delim arg"
  [delim coll] 
  (println 
    (apply str 
           (interpose "\n" (map #(apply str (interpose delim %)) coll)))))


(defn sortVector 
  "sorts a 2d vector by the given index"
  [index coll]
  (sort-by #(nth % index) coll))


(defn help 
  "custom help message for sort.core"
  [opts args banner] 
  (when (:help opts)
    (println (str "input: " (:input opts)))
    (println (str "field: " (:field opts)))
    (println (str "delim: " (:delim opts)))
    (println banner)
    (System/exit 0)))

(defn fileKind [filename]
  (let [f (File. filename)]
    (cond
      (.isFile f)      "file"
      (.isDirectory f) "directory"
      (.exists f)      "other" 
      :else            "(non-existent)" )))
\$\endgroup\$

1 Answer 1

4
+50
\$\begingroup\$

You should use namespace-qualified keywords instead of strings in the fileKind function:

(defn fileKind [filename]
  (let [f (File. filename)]
    (cond
      (.isFile f)      ::file
      (.isDirectory f) ::directory
      (.exists f)      ::other 
      :else            ::non-existent)))

... which would require the following change in the -main function:

(= (lib/fileKind input) :lib/file)
\$\endgroup\$
3
  • \$\begingroup\$ looks like you're gonna win the bounty, have anything else to say? \$\endgroup\$
    – wespiserA
    Jan 30, 2012 at 22:04
  • \$\begingroup\$ Not really, I don't think I'd do it very differently. \$\endgroup\$
    – rreillo
    Jan 31, 2012 at 9:06
  • 1
    \$\begingroup\$ I guess your usage of pmap is a bit clumsy and could lead to decreased performance for large files due to the extra overhead. You might want to take a look at this: fatvat.co.uk/2009/05/jvisualvm-and-clojure.html \$\endgroup\$
    – rreillo
    Jan 31, 2012 at 9:15

Your Answer

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

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