Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

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)" )))
share|improve this question

1 Answer 1

up vote 3 down vote accepted
+50

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

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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