Fork me on GitHub
#cljs-dev
<
2019-01-18
>
borkdude15:01:54

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) ;;> []

borkdude15:01:51

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

john15:01:41

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

john15:01:51

Will that work with spec?

borkdude15:01:52

yes, that works, I do that all the time

Alex Miller (Clojure team)15:01:55

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

Alex Miller (Clojure team)15:01:07

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

Alex Miller (Clojure team)15:01:21

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

borkdude15:01:55

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

Alex Miller (Clojure team)16:01:45

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

Alex Miller (Clojure team)16:01:00

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

Alex Miller (Clojure team)16:01:13

as “abc” is seqable but not a coll

borkdude16:01:16

ArrayList also works in CLJ

Alex Miller (Clojure team)16:01:46

well, it’s some weirder type then :)

borkdude16:01:03

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

Alex Miller (Clojure team)16:01:04

but I would say it’s not seqable

Alex Miller (Clojure team)16:01:29

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

Alex Miller (Clojure team)16:01:50

I think intent is “Clojure or Java collection”

borkdude16:01:04

is an array a Java collection?

Alex Miller (Clojure team)16:01:30

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

Alex Miller (Clojure team)16:01:41

but in an abstract sense, maybe yes

borkdude16:01:09

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

borkdude16:01:36

note that array currently doesn’t work in CLJ

Alex Miller (Clojure team)16:01:18

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

borkdude16:01:15

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

Alex Miller (Clojure team)16:01:26

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

Alex Miller (Clojure team)16:01:45

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

Alex Miller (Clojure team)16:01:06

it neither takes a seqable nor returns a seq

borkdude16:01:09

I’ll probably spec them separately:

borkdude16:01:29

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

borkdude16:01:53

@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?

borkdude16:01:33

probably low priority

Alex Miller (Clojure team)16:01:57

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

borkdude16:01:50

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

Alex Miller (Clojure team)16:01:46

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

Alex Miller (Clojure team)16:01:05

from 2010 by rich, so yeah pretty far back

borkdude17:01:18

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

Alex Miller (Clojure team)17:01:49

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