From b7e6c36d68f144b3d3d23c4d6ac991a2af92f977 Mon Sep 17 00:00:00 2001
From: Simon Brooke <simon@journeyman.cc>
Date: Wed, 26 Jun 2019 12:06:22 +0100
Subject: [PATCH] More sophisticated sanity tests all working satisfactorily

---
 project.clj                     | 19 ++++++++-
 src/sparse_array/core.clj       | 73 +++++++++++++++++++++++++++------
 test/sparse_array/core_test.clj | 43 +++++++++++++++++++
 3 files changed, 122 insertions(+), 13 deletions(-)

diff --git a/project.clj b/project.clj
index d9c47b7..fb40d19 100644
--- a/project.clj
+++ b/project.clj
@@ -3,4 +3,21 @@
   :url "http://example.com/FIXME"
   :license {:name "Eclipse Public License"
             :url "http://www.eclipse.org/legal/epl-v10.html"}
-  :dependencies [[org.clojure/clojure "1.8.0"]])
+  :dependencies [[org.clojure/clojure "1.8.0"]]
+
+  ;; `lein release` doesn't play nice with `git flow release`. Run `lein release` in the
+  ;; `develop` branch, then merge the release tag into the `master` branch.
+
+  :release-tasks [["vcs" "assert-committed"]
+                  ["clean"]
+                  ["test"]
+                  ["codox"]
+                  ["change" "version" "leiningen.release/bump-version" "release"]
+                  ["vcs" "commit"]
+                  ;; ["vcs" "tag"] -- not working, problems with secret key
+                  ["uberjar"]
+                  ["install"]
+                  ["deploy" "clojars"]
+                  ["change" "version" "leiningen.release/bump-version"]
+                  ["vcs" "commit"]])
+
diff --git a/src/sparse_array/core.clj b/src/sparse_array/core.clj
index be8a3f0..ca98565 100644
--- a/src/sparse_array/core.clj
+++ b/src/sparse_array/core.clj
@@ -1,6 +1,15 @@
 (ns sparse-array.core)
 
-(def ^:dynamic *safe-sparse-operations* false)
+(def ^:dynamic *safe-sparse-operations*
+  "Whether spase array operations should be conducted safely, with careful
+  checking of data conventions and exceptions thrown if expectations are not
+  met. Normally `false`."
+  false)
+
+(defmacro unsafe-sparse-operations?
+  "returns `true` if `*safe-sparse-operations*` is `false`, and vice versa."
+  []
+  (not (true? *safe-sparse-operations*)))
 
 (defn make-sparse-array
   "Make a sparse array with these `dimensions`. Every member of `dimensions`
@@ -16,6 +25,18 @@
                 :data
                 (rest dimensions))}))
 
+(defn- safe-test-or-throw
+  "If `v` is truthy or `*safe-sparse-operations*` is false, return `v`;
+  otherwise, throw an `ExceptionInfo` with this `message` and the map `m`."
+  [v message m]
+  (if-not
+    v
+    (if
+      *safe-sparse-operations*
+      (throw (ex-info message m))
+      v)
+    v))
+
 (defn sparse-array?
   "`true` if `x` is a sparse array conforming to the conventions established
   by this library, else `false`."
@@ -31,14 +52,27 @@
            (:content x))))))
   ([x & axes]
    (and
-     (map? x)
-     (number? (:dimensions x))
-     (pos? (:dimensions x))
-     (keyword? (:coord x))
-     (= (:coord x) (first axes))
+     (safe-test-or-throw
+       (map? x)
+       "Array must be a map" {:array x})
+     (safe-test-or-throw
+       (and (integer? (:dimensions x)) (pos? (:dimensions x)))
+       (str "The value of `:dimensions` must be a positive integer, not " (:dimensions x))
+       {:array x})
+     (safe-test-or-throw
+       (keyword? (:coord x))
+       (str "The value of `:coord` must be a keyword, not " (:coord x))
+       {:array x})
+     (safe-test-or-throw
+       (= (:coord x) (first axes))
+       (str "The value of `:coord` must be " (first axes) ", not " (:coord x))
+       {:array x})
      (if
        (empty? (rest axes))
-       (= (:content x) :data)
+       (safe-test-or-throw
+         (= (:content x) :data)
+         "If there are no further axes the value of `:content` must be `:data`"
+         {:array x})
        (and
          (= (:content x) (rest axes))
          (every?
@@ -50,7 +84,7 @@
   `coordinates`. Returns `nil` if any coordinate is invalid."
   [array value & coordinates]
   (cond
-    (and *safe-sparse-operations* (sparse-array? array))
+    (and *safe-sparse-operations* (not (sparse-array? array)))
     (throw (ex-info "Sparse array expected" {:array array}))
     (every?
       #(and (integer? %) (or (zero? %) (pos? %)))
@@ -72,16 +106,31 @@
     (throw
       (ex-info
         "Coordinates must be zero or positive integers"
-        {:coordinates coordinates
-         :invalid (remove integer? coordinates)}))))
+        {:array array
+         :coordinates coordinates
+         :invalid (remove #(and (pos? %) (integer? %)) coordinates)}))))
 
 (defn get
   "Return the value in this sparse `array` at these `coordinates`."
   ;; TODO: I am CERTAIN there is a more elegant solution to this.
   [array & coordinates]
-  (if
+  (cond
+    *safe-sparse-operations*
+    (cond
+      (not (sparse-array? array))
+      (throw (ex-info "Sparse array expected" {:array array}))
+      (not (every?
+             #(and (integer? %) (or (zero? %) (pos? %)))
+             coordinates))
+      (throw
+        (ex-info
+          "Coordinates must be zero or positive integers"
+          {:array array
+           :coordinates coordinates
+           :invalid (remove #(and (pos? %) (integer? %)) coordinates)})))
     (= :data (:content array))
     (array (first coordinates))
+    :else
     (apply get (cons (array (first coordinates)) (rest coordinates)))))
 
 (defn merge-sparse-arrays
@@ -98,7 +147,7 @@
     nil
     (= :data (:content a1))
     (merge a1 a2)
-    :else
+    (or (unsafe-sparse-operations?) (and (sparse-array? a1) (sparse-array? a2)))
     (reduce
       merge
       a2
diff --git a/test/sparse_array/core_test.clj b/test/sparse_array/core_test.clj
index b7d5707..5ec9c85 100644
--- a/test/sparse_array/core_test.clj
+++ b/test/sparse_array/core_test.clj
@@ -50,6 +50,49 @@
           "Can't have data in a non-data layer")
     ))
 
+(deftest testing-safe
+  (testing "Checking that correct exceptions are thrown when `*safe-sparse-operations*` is true"
+    (binding [*safe-sparse-operations* true]
+      (is
+        (thrown-with-msg?
+            clojure.lang.ExceptionInfo
+            #"Array must be a map"
+            (sparse-array? [])))
+      (is
+        (thrown-with-msg?
+          clojure.lang.ExceptionInfo
+          #"The value of `:dimensions` must be a positive integer, not .*"
+          (sparse-array?
+            (dissoc (make-sparse-array :x :y :z) :dimensions)))
+        "All mandatory keywords must be present: dimensions")
+    (is (thrown-with-msg?
+          clojure.lang.ExceptionInfo
+          #"The value of `:coord` must be a keyword, not .*"
+          (sparse-array?
+            (dissoc (make-sparse-array :x :y :z) :coord))))
+    (is (thrown-with-msg?
+          clojure.lang.ExceptionInfo
+          #"If there are no further axes the value of `:content` must be `:data`"
+          (sparse-array?
+            (dissoc (make-sparse-array :x :y :z) :content)))
+        "All mandatory keywords must be present: content")
+    (is (thrown-with-msg?
+            clojure.lang.ExceptionInfo
+            #"Array must be a map"
+          (sparse-array? {:dimensions 2,
+                          :coord :x,
+                          :content '(:y),
+                          3 {:dimensions 1,
+                             :coord :y,
+                             :content :data,
+                             4 "hello"},
+                          4 {:dimensions 1,
+                             :coord :y,
+                             :content :data,
+                             3 "goodbye"}
+                          5 :foo}))
+          "Can't have data in a non-data layer"))))
+
 (deftest put-and-get
   (testing "get"
     (let [expected "hello"