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\$
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\$
  • \$\begingroup\$ looks like you're gonna win the bounty, have anything else to say? \$\endgroup\$ – wespiserA Jan 30 '12 at 22:04
  • \$\begingroup\$ Not really, I don't think I'd do it very differently. \$\endgroup\$ – rreillo Jan 31 '12 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 '12 at 9:15

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.