rand-nth on empty collections

135 views
Skip to first unread message

Marc O'Morain

unread,
Sep 30, 2015, 7:38:49 AM9/30/15
to Clojure
Hi all,

I was surprised by the fact that rand-nth thrown when called on an empty collection - see http://dev.clojure.org/jira/browse/CLJ-925

This behaviour is strange, since passing nil to rand-nth returns nil, whereas in my experience, other Clojure functions treat nil and empty collections in the same way (compare with next/rest/first, etc).

http://dev.clojure.org/jira/browse/CLJ-925 was marked as Completed without comment – is this the intended behaviour? If so, would you accept a patch to update the docstring to make it clear that the function will throw an exception when passed an empty collection?

Thanks,

Marc

Alex Miller

unread,
Sep 30, 2015, 10:14:22 AM9/30/15
to Clojure
rand-nth cannot return a random element if there are no elements to return, so I do not find this surprising. Returning nil would be an invalid answer in my opinion.

The rand-nth doc string implies that rand-nth will inherit the semantics of nth for the specified collection. nth will generally throw on an invalid index (and all indexes are invalid for an empty collection).

For nil, the docstring for nth does not mention nil explicitly, so I would treat this as undefined behavior unless you consider it as a degenerate sequence. 

The code (in RT.nthFrom()) explicitly handles this case and effectively treats nil as an infinite indexed source of nils, rather than as an empty sequence. In my own opinion (but maybe not Rich's), it seems to make more sense to take the sequence interpretation and say that on nil, nth should throw instead.

So that might be a valid ticket. I'm not sure what, if any, negative consequences would arise out of existing uses of nth that rely on this behavior though. Given the number of things built on nth, it may be impossible to change this even if it made sense to.

Alex

Alex Miller

unread,
Sep 30, 2015, 10:19:51 AM9/30/15
to Clojure
Just for kicks, I built with nth throwing an exception on nil instead of returning nil and all tests passed, so that's at least promising that changing nth on nil would be possible.

Mark Engelberg

unread,
Sep 30, 2015, 3:15:11 PM9/30/15
to clojure
To me, the current behavior of nth makes sense because I know that the way you traditionally write nth on a seq / linked list is to call rest n times and then call first.  Since Clojure's rest and first on nil (or the empty list) return nil, then I would expect nth to return nil when called on an empty sequence for any n.  In other words, the behavior of nth makes complete sense in the context of first and rest returning nil (which is different from the philosophy of languages like Scheme, but is a feature people get along with just fine in Clojure).

I also think it makes perfect sense for rand-nth to throw an error on an empty collection.  That's because the first step is it needs to generate a random number between 0 and the length of the collection (0), which is impossible.  So it should throw an error.  Note that it is the *random generation of the index*, not the nth that conceptually is throwing the error.

To me, the only weird part is that rand-nth doesn't throw an error on nil. In this context, I expect nil to be treated as an empty sequence.

Clojure has other inconsistencies relating to nil, so I wouldn't generally make assumptions rand-nth's handling of nil in my code, but it's pretty easy to write your code in such a way that you are using `seq`-ified linked lists, and then later make some change so that things are getting passed around before `seq` has been called on them, which would cause rand-nth to break.  So this inconsistency is not ideal.


Mark Engelberg

unread,
Sep 30, 2015, 3:18:37 PM9/30/15
to clojure
On Wed, Sep 30, 2015 at 12:14 PM, Mark Engelberg <mark.en...@gmail.com> wrote:
I also think it makes perfect sense for rand-nth to throw an error on an empty collection.  That's because the first step is it needs to generate a random number between 0 and the length of the collection (0), which is impossible.  So it should throw an error.  Note that it is the *random generation of the index*, not the nth that conceptually is throwing the error.

To be clear, when I say that nth "conceptually is throwing the error", I just mean that's how I rationalize Clojure's behavior.  That's not really what's going on.  (rand-int 0) returns 0 (which it probably shouldn't, given that the input is meant to be an exclusive upper bound).  So in fact, the error is thrown by clojure.lang.RT.nthFrom, which is surprising.

 

Jan-Paul Bultmann

unread,
Sep 30, 2015, 4:58:13 PM9/30/15
to clo...@googlegroups.com
Throwing an exception is probably the right thing to do if it's supposed to closely model the behavior of `nth`, but it wonder if it's the most usefull behavior in practice.
I would guess that it introduces `not-empty` checks everywhere, instead of only for the cases where you want to be shure that the nil returned is actually an element of the collection.

cheers Jan
--
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clo...@googlegroups.com
Note that posts from new members are moderated - please be patient with your first post.
To unsubscribe from this group, send email to
clojure+u...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Marc O'Morain

unread,
Sep 30, 2015, 5:53:04 PM9/30/15
to clo...@googlegroups.com
I also think it makes perfect sense for rand-nth to throw an error on an empty collection.  That's because the first step is it needs to generate a random number between 0 and the length of the collection (0), which is impossible.  So it should throw an error.  Note that it is the *random generation of the index*, not the nth that conceptually is throwing the error.
​Throwing an exception doesn't feel like idiomatic Clojure to me. In my experience Clojure throws on type errors, and returns nil to indicate failure or absence. 
​first and next don't throw when you ask for non-existing elements of a collection - they return nil to indicate absense. Similarly get and get-in return nil rather than throwing when the provided key is not associated with a value in an associative data structure. 

​That's why I found it odd that rand-nth throws. 

Marc O'Morain

unread,
Sep 30, 2015, 6:11:36 PM9/30/15
to clo...@googlegroups.com
I think Mailbox app might have messed with the quoting in my last reply. I hope it makes sense. 

I'm not suggesting that the function be changed - Clojure's API compatibility between versions is amazing and a fine example of great engineering. 

Reading other people's input on this thread has made it clear to my what I find surprising - rand-nth does not behave like `first` when called on an empty collection, and in my mind asking for the first element of a sequence and asking for a random element of that sequence are similar operations. 

I do see the point of view that nth throws on out-of-bounds access, so rand-nth should too, but in that case I would expect rand-nth to throw too. 

I would like to see that subtlety mentioned in the doc string. 

Thanks Alex & all!



--

Mikera

unread,
Sep 30, 2015, 9:49:17 PM9/30/15
to Clojure
Personally I think nth is broken and should throw an exception when passed nil (which should be interpreted as an empty sequence)

=> (nth nil 10)
nil
=> (nth '() 10)
IndexOutOfBoundsException   clojure.lang.RT.nthFrom (RT.java:885)
=> (nth [] 10)
IndexOutOfBoundsException   clojure.lang.PersistentVector.arrayFor (PersistentVector.java:157)

We want to throw exceptions with invalid arguments surely? Otherwise errors just propagate and it gets harder to trace root causes of bugs.

Following this logic, rand-nth should throw an exception in all these cases too.
Reply all
Reply to author
Forward
0 new messages