I notice CLJS shuffle is more permissive than CLJ shuffle. CLJ only accepts a java.util.Collection, but CLJS accepts seqable?, so you can e.g. do:

(shuffle "foo") ;;> ["f" "o" "o"]
(shuffle nil) ;;> []

In trying to write a spec for this, I wonder if I should make the cljs spec more permissive than the clj one

yeah, how about reader conditionals? For cljc? Switch to more permissive, only when on cljs?

Will that work with spec?

yes, that works, I do that all the time

or it’s an opportunity to sync these up for portability

I think it would be reasonable to have Clojure’s shuffle take nil

I think the change for that is just a (when coll ...)

would it be reasonable for CLJ to call seq on the argument, so shuffle becomes part of the seqable in, seqable out framework?

I don’t think that was the intent in Clojure, it’s really coll?, or maybe (s/nilable coll?)

so something like (shuffle "abc") doesn’t work

as “abc” is seqable but not a coll

ArrayList also works in CLJ

well, it’s some weirder type then :)

java.util.Collection, which doesn’t have a counterpart in CLJS.

but I would say it’s not seqable

java.util.Collection is impl-focused, I’m talking about intent

I think intent is “Clojure or Java collection”

is an array a Java collection?

it’s not an instance of java.util.Collection

but in an abstract sense, maybe yes

so maybe it should be seqable, but not a string? (I have wanted such a predicate before)

note that array currently doesn’t work in CLJ

yeah

anybody can make their own seqable, and none of those would necessarily be handled

true. and if you want to handle array on CLJ you can call seq on it and it works

instance of Collection is probably the best spec for it right now

I think I would really treat this as a collection function rather than a sequence function

it neither takes a seqable nor returns a seq

I’ll probably spec them separately:

CLJ: Collection CLJS: (s/nilable (s/or coll? array?))

@alexmiller Is there interest in an issue for CLJ to support nil and arrays in shuffle, to unify it with CLJS a little bit more?

probably low priority

sure

I haven’t looked at blame, but I assume shuffle is pretty old

ok, I’ll do it after I get to finish the spec and test it on some code from the wild

looks like it was added same time as a bunch of sequence functions - group-by, flatten, partition-by, frequencies, etc and it’s used by the tests

from 2010 by rich, so yeah pretty far back

one more thing. shouldn’t (shuffle nil) return nil instead of an empty vector, since nil does not pun as an empty vector?

that’s what I would expect (or for it to not accept nil at all)