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

impl SampleRange for user types #146

Closed
budziq opened this issue May 7, 2017 · 4 comments
Closed

impl SampleRange for user types #146

budziq opened this issue May 7, 2017 · 4 comments
Labels
C-docs Documentation E-question Participation: opinions wanted

Comments

@budziq
Copy link

budziq commented May 7, 2017

Hi I would like to bump this problem already mentioned in
#114
#82

How does one exactly implement SampleRange for user types? I was just about to write a snippet for rand on https://brson.github.io/rust-cookbook/ and while implementing Rand is enough to get some random sample, I see no way to implement SampleRange due to Range being private. This pretty much defeats the purpose of SampleRange trait as the user needs to fit the values into required range by hand.

@shepmaster
Copy link
Contributor

due to Range being private.

Specifically, Range is public, but the fields of Range are not. This means that

  1. There's no reasonable way to implement construct_range, as it says:

    Construct the Range object that sample_range requires. This should not ever be called directly, only via Range::new

    Since we cannot construct Range values ourselves, this only leaves calling Range::new, which means that there's likely to be an infinite loop.

  2. There's no reasonable way to implement sample_range, as this is handed a Range, but we cannot pull out the range values to do anything interesting.

@luser
Copy link

luser commented Jul 27, 2017

Is there a reason Range exists as a type separate from std::ops::Range? I see that it has an additional accept_zone field, is that just an optimization to avoid calculating it every time sample_range is called? If std::ops::Range could be used then you could presumably have impl<T: Rand> SampleRange<T> for std::ops::Range<T>. I guess you'd also need specialization to write the integer vs. float implementations as they currently are?

@dhardy
Copy link
Member

dhardy commented Aug 17, 2017

As an alternative to making the fields of Range public, Range can have a templated inner type. This is a bit messy (since so many public types and traits are exposed) but is a valid alternative.

Depending on the performance of leading_zeros the third field could be removed. I'll try to benchmark an impl on std::ops::Range later but I suspect it'll be slower.

Edit: benchmark results here

@pitdicker
Copy link
Contributor

Implemented as part of #274.

@dhardy dhardy closed this as completed Mar 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-docs Documentation E-question Participation: opinions wanted
Projects
None yet
Development

No branches or pull requests

6 participants