Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: slices: add Shuffle #61612

Closed
hallazzang opened this issue Jul 27, 2023 · 6 comments
Closed

proposal: slices: add Shuffle #61612

hallazzang opened this issue Jul 27, 2023 · 6 comments

Comments

@hallazzang
Copy link
Contributor

hallazzang commented Jul 27, 2023

Why?

Every time I use Shuffle, I have to write swap function like this and the form of it isn't likely to change:

func(i, j int) {
	s[i], s[j] = s[j], s[i]
}

Also, there's a chance to make a typo, like: s[i], s[j] = s[i], s[i].

I think this is a good example to use generic. We also have slices.Sort, which feels like a shortcut to sort.Slice to me, so I thought it makes sense to have slices.Shuffle too.

Proposal

Add Shuffle function like this:

// Shuffle pseudo-randomizes the order of elements of the slice in place.
func Shuffle[S ~[]E, E any](r *rand.Rand, s S) {
	r.Shuffle(len(s), func(i, j int) {
		s[i], s[j] = s[j], s[i]
	})
}

We can also consider some alternatives:

  • Add the functionality to math/rand instead
  • Accept interface { Shuffle(int, func(int, int)) } instead of *rand.Rand
@gopherbot gopherbot added this to the Proposal milestone Jul 27, 2023
@ianlancetaylor
Copy link
Contributor

Thanks. You said what, but not why. When would this be used? See also https://go.dev/doc/faq#x_in_std .

@jimmyfrasche
Copy link
Member

I've used shuffle a few times but only ever with slices.

This feels like it should be a method on *rand.Rand though that would need to wait for some language changes.

One option to simplify using Shuffle today would be a generic version of reflect.Swapper:

package slices
func Swapper[S ~[]E, E any](slice S) func(i, j int) {
  return func(i, j int) {
    s[i], s[j] = s[j], s[i]
  })
}

But since package slices will have all those handy sorting functions the only real place that I can think of using this is with Shuffle.

@hallazzang
Copy link
Contributor Author

@ianlancetaylor Updated the issue. I tried to submit this issue to https://github.com/golang/exp because I thought that place is for experimental things, but I saw that the golang.org/exp/slices package is about to move into std so I put the issue here.
@jimmyfrasche Thanks, I never knew that there's reflect.Swapper. But in my opinion your version of Swapper is not much related to reflect and I don't expect reflect package will have that function in near future..

@rsc
Copy link
Contributor

rsc commented Aug 9, 2023

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Aug 16, 2023

It seems like a level inversion to me to have slices import rand. Perhaps rand/v2 (#61716) should be handling this instead. There is an active discussion about that over there.

@rsc
Copy link
Contributor

rsc commented Aug 16, 2023

This proposal is a duplicate of a previously discussed proposal, as noted above,
and there is no significant new information to justify reopening the discussion.
The issue has therefore been declined as a duplicate.
— rsc for the proposal review group

@rsc rsc closed this as completed Aug 16, 2023
@golang golang locked and limited conversation to collaborators Aug 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants