[5.1] Collection random with value 0 or 1 does not return a collection.

[lukasoppermann]

Hey,

I just notices, that the random function in a collection does not return a new collection if the argument is explicitly set to one. I would like this to work, because this way I can get a random number between 1 and x or even 0 and x and deal with the returned value the same.

With the current implementation I would have to somehow check if I got a collection or an item back.

I would love it, if $collection->random(x) where x != null would always return a collection, and $collection->random() would always return an item. This would make it much easier to work with. Thanks.

[GrahamCampbell]

What laravel versions please?

[lukasoppermann]

Sorry, actually Lumen (5.1.6) (Laravel Components 5.1.*). Is this changed in Laravel 5.2?

[GrahamCampbell]

This is actually not a bug, and is intentional.

[GrahamCampbell]

https://github.com/laravel/framework/blob/5.1/src/Illuminate/Support/Collection.php#L618

[GrahamCampbell]

The return type of that method is not collection.

[GrahamCampbell]

I do agree this behaviour is silly, and would like to see it changed. Any chance you could send a PR to the master branch (L5.3) to change this please?

[GrahamCampbell]

Hmm, actually please don't send a PR. This is very much by design.

[GrahamCampbell]

Calling ->random() will give you a random element from the collection. It would be silly to have that in a collection too.

[lukasoppermann]

I'll try my best, sure.

[GrahamCampbell]

I'll try my best, sure.

Please don't. see my later comments.

[lukasoppermann]

Random without an argument returning an item makes sense. This is similar to a Boolean $collection where no value == false.

But to have a different type of return value depending on the int seems not optimal. This just makes it complicated.

With the "new" idea a user who wants an item just calls random without and argument. If you want a collection, you provide the argument and get a collection, no matter if you provide a 0, 1, or 15.

This would make it easier to work with, without reducing functionally in any way.

[lukasoppermann]

Hey @GrahamCampbell, could you reopen this issue?

I think there is a valid reason to change this design, as other methods work like this as well:

• take(x) for example returns a collection with 0 or 1 as the value
• slice(x,y) also returns a collection if 0 or 1 as the second argument (length)
• where() always returns a collection, also if only one item is found
• splice(x,y) returns a collection if 0 or 1 as the second argument (length)

In short any method that can return one or more items, always returns a collection, no matter if 0, 1 or more items are returned. This is a common practise and sensible to use.

However, random is a mix, as the random() call with no argument is much like last() where you know only one item is returned.

But random($n) is much like the ones above, where $n could be anything from 0 to 100 and it might even be a random value within. So here you would expect a collection to be returned, no matter what number $n is, just like when randomly assigning a number to take.

Thus I would argue, it would be better for all, and easier to use, if a collection is always returned when $n is provided, while an item is always returned when no $n is provided.

[lukasoppermann]

Ping @GrahamCampbell or @taylorotwell? Any chance of this being reopened? I could send a PR.

[powelski]

I agree with @lukasoppermann. Having different return types depending on the value (not the type) is very bad design. It's also prone to bugs that are hard to find if we pass any dynamic value as the $amount parameter.

[SRautila]

This all boils down to the stupidity of PHP's array_rand() having mixed return types. I would suggest re-implementing it into two methods. One random() which always returns one element and one randomSub(n) which returns a collection of n random elements, where n can be 0 (unlike with array_rand()).

[powelski]

@SRautila Exactly! Two separate methods are way to go. And I would also love the method to pick element randomly and remove it. That would be very cool to use in loops.

[trip-somers]

I'm jumping on this train as well. I've been trying to seed my Spark database for a while and I couldn't figure out why $teams->random( random_int(1, 2) ) was "randomly" returning the entire list of team IDs in my team_users table.

Sample log output:

[2016-08-21 23:41:34] local.INFO: User ID: 1  
[2016-08-21 23:41:34] local.INFO: Team Count: 2  
[2016-08-21 23:41:34] local.INFO: Teams to Join: [24,22]  
[2016-08-21 23:41:34] local.INFO: User ID: 3  
[2016-08-21 23:41:34] local.INFO: Team Count: 1  
[2016-08-21 23:41:34] local.INFO: Teams to Join: [1,2,21,3,4,5,6,7,8,9,10,11,12,22,13,23,14,15,24,16,17,18,19,20]  

This comes from the following as I loop through my users to assign them to a pre-selected group of 4 teams: $teams_to_join = $teams->random( $team_count );

With an explicit 1 as the parameter for random(), I would expect a collection of 1. I can see how this would be a breaking change for anyone already using an explicit 1, but I can't imagine anyone doing that unless, as in this use-case, the 1 comes from a variable with a random/unknown value.

The hoops and modifications required to make this use-case work under the current implementation are insane (unless, of course, I'm overlooking something obvious?).

I think this should be re-opened.

[powelski]

With an explicit 1 as the parameter for random(), I would expect a collection of 1. I can see how this would be a breaking change for anyone already using an explicit 1, but I can't imagine anyone doing that unless, as in this use-case, the 1 comes from a variable with a random/unknown value.

Are you sure you aren't using some library that alters Eloquent's behavior? $teams->random(1) returns Eloquent Collection of size 1 for many years now. It's been fixed not long after this discussion was created, so that would be around 8 years ago.

[lucasdcrk]

With an explicit 1 as the parameter for random(), I would expect a collection of 1. I can see how this would be a breaking change for anyone already using an explicit 1, but I can't imagine anyone doing that unless, as in this use-case, the 1 comes from a variable with a random/unknown value.

Are you sure you aren't using some library that alters Eloquent's behavior? $teams->random(1) returns Eloquent Collection of size 1 for many years now. It's been fixed not long after this discussion was created, so that would be around 8 years ago.

Yup my bad, my value was null instead of 1 :)