Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## 1.1.4-SNAPSHOT

* Add SORTED, sorted, and sorted-by navs (thanks @IGJoshua)
* Add arglist metadata to navs (thanks @phronmophobic)
* Improve before-index performance by 150x on lists and 5x on vectors (thanks @jeff303)
* Bug fix: BEFORE-ELEM, AFTER-ELEM, FIRST, LAST, BEGINNING, and END on subvecs now produce vector type in cljs
Expand Down
58 changes: 58 additions & 0 deletions src/clj/com/rpl/specter.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -1504,3 +1504,61 @@
[& path]
(map compact* path)
))

(defnav
^{:doc "Navigates to a sequence resulting from (sort ...), but is a
view to the original structure that can be transformed.
If the transformed sequence is smaller than the input sequence, values
which are included are sorted by the same indices as the input value's
index in the input sequence.
If the transformed sequence is larger than the input sequence, values
added to the end of the sequence will be appended to the end of the
original sequence."}
SORTED
[]
(select* [this structure next-fn]
(n/sorted-select structure identity compare next-fn))
(transform* [this structure next-fn]
(n/sorted-transform structure identity compare next-fn)))

(defnav
^{:doc "Navigates to a sequence resulting from (sort comparator ...), but
is a view to the original structure that can be transformed.
If the transformed sequence is smaller than the input sequence, values
which are included are sorted by the same indices as the input value's
index in the input sequence.
If the transformed sequence is larger than the input sequence, values
added to the end of the sequence will be appended to the end of the
original sequence."}
sorted
[comparator]
(select* [this structure next-fn]
(n/sorted-select structure identity comparator next-fn))
(transform* [this structure next-fn]
(n/sorted-transform structure identity comparator next-fn)))

(defdynamicnav sorted-by
"Navigates to a sequence sorted by the value stored in the keypath, by the
comparator, if one is provided.
This sequence is a view to the original structure that can be transformed. If
the transformed sequence is smaller than the input sequence, values which are
included are sorted by the same indices as the input value's index in the
input sequence.
If the transformed sequence is larger than the input sequence, values added to
the end of the sequence will be appended to the end of the original sequence.
Value collection (e.g. collect, collect-one) may not be used in the keypath."
([keypath] (sorted-by keypath compare))
([keypath comparator]
(late-bound-nav [late (late-path keypath)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either the naming or implementation here is off. As named, this isn't accepting general paths but only a key. In this case, it's wrong to use late-path and it will be more straightforward/faster to use get rather than compiled-select. Additionally, compiled-select returns a sequence of results which I don't think you want to be comparing against.

An alternative is to have the first arg be an "extractor path", and use compiled-select-one! to extract the key. This would allow for usage where the desired comparator key is nested or transformed, like (sorted-by [:a :b (view -)])

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, my intention was to have it be a generic full path, what you're calling an extractor path. I thought keypath would be an appropriate term here since in clojure.core/sort-by the argument is called the keyfn. I'll change this to use compiled-select-one!.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated this to use compiled-select-one!. Would you recommend I also rename the keypath, or is this enough?

late-fn comparator]
(select* [this structure next-fn]
(n/sorted-select structure #(compiled-select late %) late-fn next-fn))
(transform* [this structure next-fn]
(n/sorted-transform structure #(compiled-select late %) late-fn next-fn)))))
10 changes: 10 additions & 0 deletions src/clj/com/rpl/specter/impl.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,16 @@
res
))))

(defn sorted-transform*
[structure keyfn comparator next-fn]
(let [sorted (sort-by (comp keyfn second) comparator (map-indexed vector structure))
indices (map first sorted)
result (next-fn (map second sorted))
unsorted (sort-by first compare (map vector (concat indices (repeat ##Inf)) result))]
(into (empty structure)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work with lists? The issue with (into '() ...) is it prepends for each element, oftentimes reversing the output order of what you expect. This needs a test at least.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a very good point, I'll add a test for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated this, it has a test and works with lists as well as vectors.

(map second)
unsorted)))

(defn- matching-indices [aseq p]
(keep-indexed (fn [i e] (if (p e) i)) aseq))

Expand Down
5 changes: 5 additions & 0 deletions src/clj/com/rpl/specter/navs.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,11 @@

(def srange-transform i/srange-transform*)

(defn sorted-select
[structure keyfn comparator next-fn]
(next-fn (sort-by keyfn comparator structure)))

(def sorted-transform i/sorted-transform*)

(defn extract-basic-filter-fn [path]
(cond (fn? path)
Expand Down
22 changes: 20 additions & 2 deletions test/com/rpl/specter/core_test.cljc
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
(ns com.rpl.specter.core-test
#?(:cljs (:require-macros
[cljs.test :refer [is deftest]]
[cljs.test :refer [is deftest testing]]
[clojure.test.check.clojure-test :refer [defspec]]
[com.rpl.specter.cljs-test-helpers :refer [for-all+]]
[com.rpl.specter.test-helpers :refer [ic-test]]
Expand All @@ -13,7 +13,7 @@
defdynamicnav traverse-all satisfies-protpath? end-fn
vtransform]]))
(:use
#?(:clj [clojure.test :only [deftest is]])
#?(:clj [clojure.test :only [deftest is testing]])
#?(:clj [clojure.test.check.clojure-test :only [defspec]])
#?(:clj [com.rpl.specter.test-helpers :only [for-all+ ic-test]])
#?(:clj [com.rpl.specter
Expand Down Expand Up @@ -1702,3 +1702,21 @@
(is (satisfies-protpath? FooPP "a"))
(is (not (satisfies-protpath? FooPP 1)))
)))

(deftest sorted-test
(let [initial-list [3 4 2 1]]
(testing "the SORTED navigator"
(is (= (sort initial-list) (select-one s/SORTED initial-list)))
(is (= [2 1 3 4] (transform s/SORTED reverse initial-list)))
(is (= [3 2 1] (transform s/SORTED butlast initial-list)))
(is (= [3 5 2 1] (setval [s/SORTED s/LAST] 5 initial-list))))
(testing "the sorted navigator with comparator"
(let [reverse-comparator (comp - compare)]
(is (= (sort reverse-comparator initial-list)
(select-one (s/sorted reverse-comparator) initial-list)))
(is (= 4 (select-one [(s/sorted reverse-comparator) s/FIRST] initial-list))))))
(testing "the sorted-by navigator with keypath"
(let [initial-list [{:a 3} {:a 4} {:a 2} {:a 1}]]
(is (= (sort-by :a initial-list)
(select-one (s/sorted-by :a) initial-list)))
(is (= {:a 4} (select-one [(s/sorted-by :a (comp - compare)) s/FIRST] initial-list))))))