core.async

alts! assumes nth works on ports argument

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

Description

The docstring for alts! reads:

ports is a set of channel endpoints, which
can be either a channel to take from or a vector of
[channel-to-put-to val-to-put], in any combination.

However, trying to use an actual set yields the following exception:

java.lang.UnsupportedOperationException: nth not supported on this type: PersistentHashSet

Activity

Hide
Timothy Baldridge added a comment -

fixed in master

Show
Timothy Baldridge added a comment - fixed in master
Hide
Brandon Bloom added a comment -

> alts requires an efficiently nth-able collection

Is that the case? Or does alts actually only require efficiently sampling a single element from a collection?

Efficient nth is a reasonable proxy, since the only choice we have for randomness is rand-int. However, one could imagine a CollSample protocol that would allow efficient random choice leveraging data-structure-specific knowledge, much as CollReduce works. Given the world as it exists now, I agree that this is a doc bug.

> If CLJS supports nth on sets it is broken.

That's the conclusion we came to in IRC. David quickly fixed this:
https://github.com/clojure/clojurescript/commit/a113b08a8c2811b0590cc6a36b2e9e5adc1c4c1e

Show
Brandon Bloom added a comment - > alts requires an efficiently nth-able collection Is that the case? Or does alts actually only require efficiently sampling a single element from a collection? Efficient nth is a reasonable proxy, since the only choice we have for randomness is rand-int. However, one could imagine a CollSample protocol that would allow efficient random choice leveraging data-structure-specific knowledge, much as CollReduce works. Given the world as it exists now, I agree that this is a doc bug. > If CLJS supports nth on sets it is broken. That's the conclusion we came to in IRC. David quickly fixed this: https://github.com/clojure/clojurescript/commit/a113b08a8c2811b0590cc6a36b2e9e5adc1c4c1e
Hide
Rich Hickey added a comment -

The documentation should be changed. alts requires an efficiently nth-able collection (e.g. a vector), else it will be slow.

https://github.com/clojure/core.async/blob/master/src/main/clojure/clojure/core/async.clj#L183

If CLJS supports nth on sets it is broken.

Show
Rich Hickey added a comment - The documentation should be changed. alts requires an efficiently nth-able collection (e.g. a vector), else it will be slow. https://github.com/clojure/core.async/blob/master/src/main/clojure/clojure/core/async.clj#L183 If CLJS supports nth on sets it is broken.
Hide
Brandon Bloom added a comment -
Show
Brandon Bloom added a comment - Discussed at length in IRC on Aug 7th: http://www.raynes.me/logs/irc.freenode.net/clojure/2013-08-07.txt

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: