Skip to content

Commit

Permalink
Fix for mapcat, issue #1556
Browse files Browse the repository at this point in the history
Corrected mapcat to actually concat its results rather than merging.
Added warning to docstring thta this might not be ideal.

Added flatmap to cover map+merge case.
  • Loading branch information
daveray committed Aug 12, 2014
1 parent 478ea19 commit 387e84d
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 3 deletions.
33 changes: 31 additions & 2 deletions src/main/clojure/rx/lang/clojure/core.clj
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
(:refer-clojure :exclude [concat cons count cycle
distinct do drop drop-while
empty every?
filter first future
filter first future flatmap
group-by
interleave interpose into iterate
keep keep-indexed
Expand Down Expand Up @@ -616,17 +616,46 @@
return an Observable. The resulting observables are concatentated together
into one observable.
WARNING: This operator, like clojure.core/mapcat, preserves ordering of the
generated Observables. In an asynchronous context, this may cause unintended
blocking. Try flatmap instead.
If multiple Observables are given, the arguments to f are the first item from
each observable, then the second item, etc.
See:
clojure.core/mapcat
rx.Observable/flatMap
flatmap
rx.Observable/concatMap
"
[f & xs]
(if (clojure.core/next xs)
(mapcat* f (seq->o xs))
; use built-in flatMap for single-arg case
(.concatMap ^Observable (clojure.core/first xs) (iop/fn* f))))

(defn ^Observable flatmap*
"Same as multi-arg flatmap, but input is an Observable of Observables.
See:
flatmap
"
[f ^Observable xs]
(->> xs
(map* f)
(merge*)))

(defn ^Observable flatmap
"Like mapcat, but the Observables produced by f are merged rather than concatenated.
This behavior is preferable in asynchronous contexts where order is not important.
See:
mapcat
rx.Observable/flatMap
"
[f & xs]
(if (clojure.core/next xs)
(flatmap* f (seq->o xs))
(.flatMap ^Observable (clojure.core/first xs) (iop/fn* f))))

(defn map-indexed
Expand Down
40 changes: 39 additions & 1 deletion src/test/clojure/rx/lang/clojure/core_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,8 @@
(let [xs [{:k :a :v 1} {:k :b :v 2} {:k :a :v 3} {:k :c :v 4}]]
(testing "with just a key-fn"
(is (= [[:a {:k :a :v 1}]
[:b {:k :b :v 2}]
[:a {:k :a :v 3}]
[:b {:k :b :v 2}]
[:c {:k :c :v 4}]]
(->> xs
(rx/seq->o)
Expand Down Expand Up @@ -452,6 +452,44 @@
(rx/seq->o as)
(rx/seq->o bs)))))))

(deftest test-flatmap
(let [f (fn [v] [v (* v v)])
xs (range 10)]
(is (= (mapcat f xs)
(b/into [] (rx/flatmap (comp rx/seq->o f) (rx/seq->o xs))))))

; group-by is a good way to test merge behavior without truly async code
; here the :a and :b observables are interleaved when merged
(let [xs [{:k :a :v 1} {:k :b :v 2} {:k :a :v 3} {:k :c :v 4}]]
(is (= [[:a {:k :a :v 1}]
[:b {:k :b :v 2}]
[:a {:k :a :v 3}]
[:c {:k :c :v 4}]]
(->> xs
(rx/seq->o)
(rx/group-by :k)
(rx/flatmap (fn [[k vo :as me]]
(is (instance? clojure.lang.MapEntry me))
(rx/map #(vector k %) vo)))
(b/into [])))))

; still looking for a simple demo of merging for the multi-arg case
; Here, because ys is "inline", the interleaving is removed. sigh.
(let [xs [{:k :a :v 1} {:k :b :v 2} {:k :a :v 3} {:k :c :v 4}]
ys [:ay :by :cy]]
(is (= [[:a {:k :a :v 1} :ay]
[:a {:k :a :v 3} :ay]
[:b {:k :b :v 2} :by]
[:c {:k :c :v 4} :cy]]
(->> (rx/flatmap (fn [[k vo :as me] y]
(is (instance? clojure.lang.MapEntry me))
(rx/map #(vector k % y) vo))
(->> xs
rx/seq->o
(rx/group-by :k))
(rx/seq->o ys))
(b/into []))))))

(deftest test-next
(let [in [:q :r :s :t :u]]
(is (= (next in) (b/into [] (rx/next (rx/seq->o in)))))))
Expand Down

0 comments on commit 387e84d

Please sign in to comment.