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

Remove Distribution::sample_iter #455

Closed
sicking opened this issue May 15, 2018 · 3 comments
Closed

Remove Distribution::sample_iter #455

sicking opened this issue May 15, 2018 · 3 comments

Comments

@sicking
Copy link
Contributor

sicking commented May 15, 2018

I might be misunderstanding the purpose of Distribution::sample_iter, in which case feel free to close this issue.

Best I can tell, the Distribution::sample_iter function is not useful. Implementations of the Distribution trait are required to still return a DistIter struct, which means that the only sensible implementation of Distribution::sample_iter is to use the default implementation.

So we might as well move the implementation of Distribution::sample_iter into Rng::sample_iter and get rid of Distribution::sample_iter.

This is arguably somewhat similar to issue 451, in that we currently have two ways of creating an iterator, Uniform::new(1, 30).sample_iter(rng) and rng.sample_iter(Uniform::new(1, 30)), whereas it seems having one would be better.

I'm happy to create a pull request if there's agreement.

@pitdicker
Copy link
Contributor

In this case I like having both (both were at the same time actually). See #361.

@dhardy
Copy link
Member

dhardy commented May 15, 2018

You mean not useful the way functions like Iterator::count are trivial to implement and have no sensible implementation besides the default? Or just because there are two functions for the same thing?

I agree with @pitdicker, in part because of symmetry with Rng::sample and Distribution::sample. It's weird for sure, but there's isn't really a good alternative I don't think (even the old Rand could be used in two ways).

@dhardy dhardy closed this as completed May 15, 2018
@sicking
Copy link
Contributor Author

sicking commented May 15, 2018

Not useful in the sense that implementations of the trait can't provide a faster or otherwise more optimal implementation of the function.

But if the preference is to keep the function then so be it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants