From 9027f7f248ec843a32808e98dc9f6fa8c8335b0c Mon Sep 17 00:00:00 2001
From: Jeff Evans <jeffrey.wayne.evans@gmail.com>
Date: Mon, 3 Aug 2020 13:19:09 -0500
Subject: [PATCH] Improve before-index performance #223

Adding new protocol for performing the insert-before-idx operation, with
implementations for core collection types

Adding new functional test to confirm behavior when operating on a string

Adding benchmarks to compare new performance vs old implementation, and
also against cons at beginning of list
---
 scripts/benchmarks.clj              | 16 +++++++++++++++
 src/clj/com/rpl/specter.cljc        |  9 ++++----
 src/clj/com/rpl/specter/navs.cljc   | 32 +++++++++++++++++++++++++++++
 test/com/rpl/specter/core_test.cljc |  6 +++++-
 4 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/scripts/benchmarks.clj b/scripts/benchmarks.clj
index 84bc1be3..072ecf93 100644
--- a/scripts/benchmarks.clj
+++ b/scripts/benchmarks.clj
@@ -335,3 +335,19 @@
     (transform (walker number?) inc data)
     (transform (walker-old number?) inc data)
     ))
+
+(let [size 1000
+      middle-idx (/ size 2)
+      v -1
+      rng (range size)
+      data-vec (vec rng)
+      data-lst (apply list rng)]
+  (run-benchmark "before-index vs. srange (vector)"
+    (setval (before-index middle-idx) v data-vec)
+    (setval (srange middle-idx middle-idx) [v] data-vec))
+  (run-benchmark "before-index vs. srange (list)"
+    (setval (before-index middle-idx) v data-lst)
+    (setval (srange middle-idx middle-idx) [v] data-lst))
+  (run-benchmark "before-index at 0 vs. cons (list)"
+    (setval (before-index 0) v data-lst)
+    (cons v data-lst)))
diff --git a/src/clj/com/rpl/specter.cljc b/src/clj/com/rpl/specter.cljc
index 8d763262..3177a176 100644
--- a/src/clj/com/rpl/specter.cljc
+++ b/src/clj/com/rpl/specter.cljc
@@ -974,11 +974,10 @@
     NONE)
   (transform* [this vals structure next-fn]
     (let [v (next-fn vals NONE)]
-      (if (identical? NONE v)
-        structure
-        ;; TODO: make a more efficient impl
-        (setval (srange index index) [v] structure)
-        ))))
+      (if
+        (identical? NONE v)
+          structure
+          (n/insert-before-idx structure index v)))))
 
 (defrichnav
   ^{:doc "Navigates to the index of the sequence if within 0 and size. Transforms move element
diff --git a/src/clj/com/rpl/specter/navs.cljc b/src/clj/com/rpl/specter/navs.cljc
index abe0c655..f25f1053 100644
--- a/src/clj/com/rpl/specter/navs.cljc
+++ b/src/clj/com/rpl/specter/navs.cljc
@@ -474,6 +474,9 @@
 (defprotocol FastEmpty
   (fast-empty? [s]))
 
+(defprotocol InsertBeforeIndex
+  (insert-before-idx [aseq idx val]))
+
 (defnav PosNavigator [getter updater]
   (select* [this structure next-fn]
     (if-not (fast-empty? structure)
@@ -691,3 +694,32 @@
     ((:end-fn end-fn) structure start)
     (end-fn structure)
     ))
+
+(defn- insert-before-index-list [lst idx val]
+  ;; an implementation that is most efficient for list style structures
+  (let [[front back] (split-at idx lst)]
+    (concat front (cons val back))))
+
+(extend-protocol InsertBeforeIndex
+  nil
+  (insert-before-idx [_ idx val]
+    (cond (= 0 idx) [val]
+          :else (i/throw-illegal "For a nil structure, can only insert before index 0, not at - " idx)))
+
+  #?(:clj java.lang.String :cljs string)
+  (insert-before-idx [aseq idx val]
+    (apply str (insert-before-index-list aseq idx val)))
+
+  #?(:clj clojure.lang.LazySeq :cljs cljs.core/LazySeq)
+  (insert-before-idx [aseq idx val]
+    (insert-before-index-list aseq idx val))
+
+  #?(:clj clojure.lang.IPersistentVector :cljs cljs.core/PersistentVector)
+  (insert-before-idx [aseq idx val]
+    (let [front (subvec aseq 0 idx)
+          back (subvec aseq idx)]
+      (into (conj front val) back)))
+
+  #?(:clj clojure.lang.IPersistentList :cljs cljs.core/List)
+  (insert-before-idx [aseq idx val]
+    (insert-before-index-list aseq idx val)))
diff --git a/test/com/rpl/specter/core_test.cljc b/test/com/rpl/specter/core_test.cljc
index 740bf3e3..bb4d0234 100644
--- a/test/com/rpl/specter/core_test.cljc
+++ b/test/com/rpl/specter/core_test.cljc
@@ -1609,14 +1609,18 @@
 
 (deftest before-index-test
   (let [data [1 2 3]
-        datal '(1 2 3)]
+        datal '(1 2 3)
+        data-str "abcdef"]
     (is (predand= vector? [:a 1 2 3] (setval (s/before-index 0) :a data)))
     (is (predand= vector? [1 2 3] (setval (s/before-index 1) s/NONE data)))
     (is (predand= vector? [1 :a 2 3] (setval (s/before-index 1) :a data)))
     (is (predand= vector? [1 2 3 :a] (setval (s/before-index 3) :a data)))
+    ; ensure inserting at index 0 in nil structure works, as in previous impl
+    (is (predand= vector? '[:a] (setval (s/before-index 0) :a nil)))
     (is (predand= list? '(:a 1 2 3) (setval (s/before-index 0) :a datal)))
     (is (predand= list? '(1 :a 2 3) (setval (s/before-index 1) :a datal)))
     (is (predand= list? '(1 2 3 :a) (setval (s/before-index 3) :a datal)))
+    (is (predand= string? "abcxdef" (setval (s/before-index 3) (char \x) data-str)))
     ))
 
 (deftest index-nav-test