4
\$\begingroup\$

I wrote a couple of basic Clojure functions (practicing a declarative style) and I was looking to get some feedback on them. You can see all the code here. Is there anything I could improve on to write more functional style code? How could I improve my tests? (Code can be run with lein test if you have Leiningen installed.)

Here are the main parts:

core_test.clj

(ns image-array-util.core-test
  (:require [clojure.test :refer :all]
            [image-array-util.core :refer :all]))


(deftest rotate-image-test
  (testing "Rotate an image"
    (is (= (rotate-image [[1]] :90) [[1]]))
    (is (= (rotate-image [[[0 0 0]]] :90) [[[0 0 0]]]))
    (is (= (rotate-image [[1 2]] :90) [[2][1]]))
    (is (= (rotate-image [[[1 1 1][2 2 2]]] :90) [[[2 2 2]][[1 1 1]]]))
    (is (= (rotate-image [[1 2]] :180) [[2 1]]))
    (is (= (rotate-image [[[1 1 1][2 2 2]]] :180) [[[2 2 2][1 1 1]]]))
    (is (= (rotate-image [[[1 1 1][2 2 2][3 3 3]][[4 4 4][5 5 5][6 6 6]]] :270) [[ [4 4 4] [1 1 1]][[5 5 5][2 2 2]][[6 6 6][3 3 3]]]))
    (is (thrown-with-msg? Exception #"Can't do that bucco. Argument 'deg' \(curently ':1'\) must be one of :90 :180 :270." (rotate-image [[1]] :1)))))

(deftest crop-image-test
  (testing "Cropping an image"
    (is (= (crop-image [[1]] [0 1] [0 1]) [[1]]))
    (is (= (crop-image [[1 2 3][4 5 6][7 8 9]] [0 1] [0 3]) [[1 2 3]]))
    (is (= (crop-image [[1 2 3][4 5 6][7 8 9]] [0 3] [0 1]) [[1][4][7]]))
    (is (= (crop-image [[1 2 3][4 5 6][7 8 9]] [1 2] [1 2]) [[5]]))
    (is (= (crop-image [[ [1 1 1] [2 2 2] [3 3 3]]
                        [ [4 4 4] [5 5 5] [6 6 6]]
                        [ [7 7 7] [8 8 8] [9 9 9]]] [1 2] [1 2]) [[[5 5 5]]]))))

core.clj

(ns image-array-util.core)

(defn rotate-image [image deg]
  (case deg
    :90 (let [new-height (count (nth image 0)) new-width (count image)]
          (->>
            (map reverse image)
            (apply interleave)
            (partition new-width)))
    :180 (reverse (map reverse image))
    :270 (rotate-image (rotate-image image :90) :180)
    (throw (IllegalArgumentException. (str "Can't do that bucco. Argument 'deg' (curently '" deg "') must be one of :90 :180 :270.")))))

(defn crop-image [image [vertical-s vertical-e] [horizontal-s horizontal-e]]
  (map #(subvec % horizontal-s horizontal-e) (subvec image vertical-s vertical-e)))
\$\endgroup\$
  • 1
    \$\begingroup\$ You could make your tests a little more concise using the are macro instead of repeating is. \$\endgroup\$ – tsleyson Jun 20 '15 at 22:20
  • \$\begingroup\$ I suggest adding tests also for the edge/invalid cases, like empty images, cropping at negative values, etc. \$\endgroup\$ – Attilio Jul 22 '15 at 19:38
  • \$\begingroup\$ Use 90, 180, 270 instead of :90 :180 :270 \$\endgroup\$ – Timothy Pratley Nov 25 '15 at 22:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.