Skip to content

Commit

Permalink
Improve combine to not return lazy sequences
Browse files Browse the repository at this point in the history
The `combine` function attempts to add new values to a collection by
merging them. However, for lists and a vectors, a lazy sequence is
returned rather than a list or a vector.

For lists, `(apply list (concat ...` is used instead of `(into ...` in
order to preserve element order for backwards compatibility, as `into`
would put the new elements at the head of the list.

For vectors, `into` is used, as new elements are added to the tail.

The implementation for sets is simplified.
  • Loading branch information
jeremyheiler committed Jan 5, 2019
1 parent 5382a89 commit 576f64a
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGES.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
version which blocked updating some dependencies
* Update clojure versions in the build matrix.
* Allow `defresource` to have a docstring (#305)
* Improve `liberator.util/combine` to not return lazy sequences (#304)

## Bugs fixed

Expand Down
6 changes: 3 additions & 3 deletions src/liberator/util.clj
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@
(cond
(-> newval meta :replace) newval
(and (map? curr) (map? newval)) (merge-with combine curr newval)
(and (list? curr) (coll? newval)) (concat curr newval)
(and (vector? curr) (coll? newval)) (concat curr newval)
(and (set? curr) (coll? newval)) (set (concat curr newval))
(and (list? curr) (coll? newval)) (apply list (concat curr newval))
(and (vector? curr) (coll? newval)) (into curr newval)
(and (set? curr) (coll? newval)) (into curr newval)
:otherwise newval))

(defn is-protocol-exception?
Expand Down
4 changes: 4 additions & 0 deletions test/test_util.clj
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@
(facts "combine function"
(facts "simple combinations"
(fact "merges map" (combine {:a 1} {:b 2}) => {:a 1 :b 2})
(fact "returns a map" (combine {:a 1} {:b 2}) => map?)
(fact "concats list" (combine '(1 2) [3 4]) => '(1 2 3 4))
(fact "returns a list" (combine '(1 2) [3 4]) => list?)
(fact "concats vector" (combine [1 2] '(3 4)) => [1 2 3 4])
(fact "returns a vector" (combine [1 2] '(3 4)) => vector?)
(fact "concats set" (combine #{1 2} [3 4]) => #{1 2 3 4})
(fact "returns a set" (combine #{1 2} [3 4]) => set?)
(facts "replaces other types"
(fact (combine 123 456) => 456)
(fact (combine "abc" 123) => 123)
Expand Down

0 comments on commit 576f64a

Please sign in to comment.